From 774650800352e2b4835beac836724fc6da87e2c9 Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 14:34:22 +0800 Subject: [PATCH 01/30] docs: design fix for issue 24784 time functions --- ...07-16-issue-24784-time-functions-design.md | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md diff --git a/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md b/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md new file mode 100644 index 0000000000000..1c43495fa0c99 --- /dev/null +++ b/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md @@ -0,0 +1,132 @@ +# Issue #24784: MySQL-Compatible TIME Function Semantics + +## Context + +Issue #24784 reports three incorrect behaviors in the SQL function layer: + +- `HOUR(TIME)` folds hours with `% 24`. +- `TIME_FORMAT(TIME, "%T")` folds hours with `% 24`. +- `MAKETIME(hour, minute, second)` discards fractional seconds. + +Investigation also found two directly related compatibility gaps: `TIME_FORMAT` +drops the sign of negative `TIME` values, and `MAKETIME` rejects negative hours +that MySQL accepts. This change fixes all five behaviors on `main`. + +## Compatibility Contract + +The implementation will follow these rules: + +1. `HOUR(TIME)` returns the absolute hour component without folding it into a + 24-hour clock. This matches MySQL, which ignores the sign when extracting the + hour. +2. The `TIME` overload of `HOUR` returns `uint32`. MatrixOne's largest internal + `TIME` hour, `types.MaxHourInTime` (`2,562,047,787`), fits in `uint32`, so the + return type cannot truncate any valid MatrixOne `TIME` value. +3. `TIME_FORMAT` prefixes one minus sign for a negative `TIME`. The sign applies + to the complete formatted result, matching MySQL's formatting flow. +4. `%H`, `%k`, and `%T` preserve the complete hour component. Twelve-hour + specifiers (`%h`, `%I`, `%l`, `%p`, and `%r`) continue to use the hour modulo + 24 because they describe a time-of-day representation. +5. `MAKETIME` accepts hours from `-838` through `838`, minutes from `0` through + `59`, and seconds from `0` up to but not including `60`. Negative hours create + a negative `TIME`. +6. Fractional seconds are rounded to MatrixOne's six-digit microsecond storage + precision. A rounded value that reaches one complete second carries into the + next minute or hour. +7. The result `TIME` scale is inherited from the second argument's numeric scale + and clamped to `0..6`. Integer seconds therefore produce scale 0, while a + value such as `56.789012` produces scale 6. +8. `NaN`, positive or negative infinity, negative seconds, invalid minutes, and + values whose normalized result exceeds `838:59:59.999999` return `NULL`. +9. Existing out-of-range policy remains unchanged: this work does not add + MySQL-style warning emission or saturation for results beyond the supported + `MAKETIME` range. + +## Design + +### HOUR(TIME) + +`TimeToHour` will use `ClockFormat` and return its hour as `uint32`, without +`% 24`. The function registry will declare only the `TIME` overload as +`uint32`; timestamp and datetime overloads remain `uint8` because their hour is +always in `0..23`. + +The sign returned by `ClockFormat` is intentionally ignored. This is a +documented compatibility choice, not an omission. + +### TIME_FORMAT + +`timeFormat` will retain the sign returned by `ClockFormat` and write `-` to the +output buffer before processing the format string. This keeps sign handling in +one place and avoids duplicating it across individual specifiers. + +The `%T` branch in `makeTimeFormat` will format the complete hour directly. +Existing `%H` and `%k` behavior already preserves the complete hour and will not +change. Twelve-hour branches will remain unchanged. + +### MAKETIME Fraction Conversion + +The second-argument extractor will return a normalized representation containing +whole seconds and microseconds. Integer inputs produce zero microseconds. Float +inputs will: + +1. reject non-finite values and values outside `[0, 60)`; +2. round `value * types.MicroSecsPerSec` to the nearest integer microsecond; +3. split the result into whole seconds and microseconds. + +Splitting after rounding naturally represents a carry as 60 whole seconds and +zero microseconds. Construction will use total duration semantics through +`types.TimeFromClock`, which normalizes the carry. The normalized value is then +validated against the supported `MAKETIME` range before it is appended. + +The hour helper will determine the sign, take the absolute hour after range +validation, and pass both to `types.TimeFromClock`. Range validation occurs +before absolute-value conversion, so there is no `math.MinInt64` overflow path. + +### Result Precision + +Each `MAKETIME` overload will use a shared return-type helper. It will copy the +third parameter's scale, clamp negative values to 0 and values above 6 to 6, and +return `types.New(types.T_time, 0, scale)`. + +The planner preserves source numeric scale when it inserts an implicit cast to +`float64`, so decimal literals such as `56.789012` retain scale 6 through overload +resolution. This allows frontend serialization to call `Time.String2(6)` and +display the stored fraction. + +## Error and Boundary Handling + +- NULL propagation remains handled by the strict function contract and the + existing vector null checks. +- Invalid rows append a NULL result without aborting evaluation of the batch. +- Floating-point validation happens before conversion to integers, preventing + implementation-defined-looking results for `NaN` and infinity. +- A fractional carry is normalized before final range validation. +- No new allocations, goroutines, locks, waits, or external resources are + introduced. + +## Testing Strategy + +Implementation will follow red-green-refactor: + +1. Extend `TestHour` with hours above 23, a negative `TIME`, and + `types.MaxHourInTime`; assert the `uint32` result type. +2. Extend `TestTimeFormat` with a large-hour `%T` case and negative formats that + verify a single leading sign. +3. Add focused `MAKETIME` unit cases for six-digit fractions, negative hours, + fractional carry, NULLs, invalid numeric values, and the upper boundary. +4. Add function-resolution tests proving that integer seconds return `TIME(0)` + and a scale-6 numeric second returns `TIME(6)` after implicit casting. +5. Update distributed `MAKETIME`, `TIME_FORMAT`, and HOUR cases and expected + results to exercise SQL-visible formatting. +6. Prove the regression tests fail without the implementation and pass with it. +7. Run formatting, package build, `go vet`, package tests with `-count=1`, a + dependent-package regression test, and relevant race tests if the test + environment supports them. + +## Scope Boundaries + +This change does not redesign MatrixOne's `TIME` representation, add new decimal +execution kernels, change time-zone behavior, or implement MySQL warning and +saturation behavior for out-of-range `MAKETIME` values. + From 09b25ac0b8603328d7692507a0ae9368cdc12a35 Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 14:38:25 +0800 Subject: [PATCH 02/30] docs: add issue 24784 implementation plan --- .../2026-07-16-issue-24784-time-functions.md | 285 ++++++++++++++++++ 1 file changed, 285 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md diff --git a/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md b/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md new file mode 100644 index 0000000000000..62c6d87f127f1 --- /dev/null +++ b/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md @@ -0,0 +1,285 @@ +# Issue #24784 TIME Functions Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `HOUR(TIME)`, `TIME_FORMAT`, and `MAKETIME` preserve large hours, signs, fractional seconds, and SQL-visible precision. + +**Architecture:** Keep the fix in the existing SQL function registry and vectorized execution functions. Widen only the TIME overload of HOUR, centralize sign handling in `timeFormat`, and normalize MAKETIME seconds to whole seconds plus microseconds before constructing `types.Time`. + +**Tech Stack:** Go, MatrixOne vectorized SQL functions, `types.Time`, `testify`, distributed SQL test files. + +## Global Constraints + +- Work from `upstream/main` in the isolated `codex/fix-24784-time-functions` worktree. +- HOUR must cover every value through `types.MaxHourInTime` and ignore the TIME sign. +- TIME_FORMAT must emit one leading minus sign for negative TIME values. +- MAKETIME accepts hours in `[-838,838]`, minutes in `[0,59]`, and seconds in `[0,60)`. +- Round fractions to at most six microsecond digits and normalize carries. +- Invalid or out-of-range MAKETIME rows append NULL; warnings and saturation are out of scope. +- Add no dependencies and do not restructure the existing large function files. + +--- + +### Task 1: Preserve Complete HOUR(TIME) Values + +**Files:** +- Modify: `pkg/sql/plan/function/func_unary_test.go` +- Modify: `pkg/sql/plan/function/func_unary.go` +- Modify: `pkg/sql/plan/function/list_builtIn.go` + +**Interfaces:** +- Consumes: `types.Time.ClockFormat()`. +- Produces: TIME overload results as `uint32`; HOUR overload 2 returns `types.T_uint32`. + +- [ ] **Step 1: Write the failing test** + +Extend the TIME case in `initHourTestCase` with positive and negative 272-hour values and `types.MaxHourInTime`. Its expected result is: + +```go +NewFunctionTestResult(types.T_uint32.ToType(), false, + []uint32{15, 0, 23, 272, 272, uint32(types.MaxHourInTime)}, + []bool{false, false, false, false, false, false}) +``` + +- [ ] **Step 2: Run RED** + +Run the verification test command with `-run '^TestHour$'`. Expect a type mismatch and the 272-hour value folded to 16. + +- [ ] **Step 3: Implement the minimal fix** + +```go +func TimeToHour(ivecs []*vector.Vector, result vector.FunctionResultWrapper, proc *process.Process, length int, selectList *FunctionSelectList) error { + return opUnaryFixedToFixed[types.Time, uint32](ivecs, result, proc, length, func(v types.Time) uint32 { + hour, _, _, _, _ := v.ClockFormat() + return uint32(hour) + }, selectList) +} +``` + +Change only HOUR overload 2 in `list_builtIn.go` to `types.T_uint32.ToType()`. + +- [ ] **Step 4: Run GREEN** + +Run `TestHour` again and expect PASS. + +- [ ] **Step 5: Commit** + +```bash +git add pkg/sql/plan/function/func_unary.go pkg/sql/plan/function/func_unary_test.go pkg/sql/plan/function/list_builtIn.go +git commit -m "fix: preserve complete time hours" +``` + +### Task 2: Preserve TIME_FORMAT Hours and Signs + +**Files:** +- Modify: `pkg/sql/plan/function/func_binary_test.go` +- Modify: `pkg/sql/plan/function/func_binary.go` + +**Interfaces:** +- Consumes: `ClockFormat` including `isNeg`. +- Produces: one leading sign and a full-hour `%T` rendering. + +- [ ] **Step 1: Write the failing tests** + +Extend the `%T` case with `123:45:06` and `-123:45:06`, expecting: + +```go +[]string{"15:30:45", "123:45:06", "-123:45:06"} +``` + +Add a negative value with format `elapsed=%H:%i:%s`, expecting `-elapsed=123:45:06`. + +- [ ] **Step 2: Run RED** + +Run the verification test command with `-run '^TestTimeFormat$'`. Expect `%T` to produce `03:45:06` and the sign to be absent. + +- [ ] **Step 3: Implement sign and hour handling** + +At the start of `timeFormat`: + +```go +hour, minute, sec, msec, isNeg := t.ClockFormat() +if isNeg { + buf.WriteByte('-') +} +``` + +Change `%T` to: + +```go +fmt.Fprintf(buf, "%02d:%02d:%02d", hour, minute, sec) +``` + +- [ ] **Step 4: Run GREEN** + +Run `TestTimeFormat` again and expect PASS. + +- [ ] **Step 5: Commit** + +```bash +git add pkg/sql/plan/function/func_binary.go pkg/sql/plan/function/func_binary_test.go +git commit -m "fix: preserve time format hours and signs" +``` + +### Task 3: Preserve MAKETIME Fractions, Scale, Carries, and Negative Hours + +**Files:** +- Modify: `pkg/sql/plan/function/func_binary_test.go` +- Modify: `pkg/sql/plan/function/function_test.go` +- Modify: `pkg/sql/plan/function/func_binary.go` +- Modify: `pkg/sql/plan/function/list_builtIn.go` + +**Interfaces:** +- Produces: `makeTimeReturnType(parameters []types.Type) types.Type`. +- Produces: `makeTimeFromInt64(hour, minute, second int64, microsecond uint32, rs *vector.FunctionResult[types.Time], i uint64) error`. +- Produces: second extractors returning `(int64, uint32, bool)`. + +- [ ] **Step 1: Write failing execution tests** + +Add `TestMakeTimeFractionAndSign` with float64 scale-6 vectors for these rows: + +```go +hours := []float64{12, -12, 12, 838, 12, 12} +minutes := []float64{34, 34, 59, 59, 34, 34} +seconds := []float64{56.789012, 56.789012, 59.9999996, 59.9999996, math.NaN(), math.Inf(1)} +expected := []types.Time{ + types.TimeFromClock(false, 12, 34, 56, 789012), + types.TimeFromClock(true, 12, 34, 56, 789012), + types.TimeFromClock(false, 13, 0, 0, 0), 0, 0, 0, +} +nulls := []bool{false, false, false, true, true, true} +``` + +Use a TIME(6) expected result. + +- [ ] **Step 2: Write failing return-scale tests** + +Add `TestMakeTimeReturnScale` in `function_test.go`. Assert all-int64 arguments return TIME(0). Resolve arguments `(int64, int64, decimal128(20,6))`; assert an implicit cast is requested and the result is TIME(6). + +- [ ] **Step 3: Run RED** + +Run with `-run 'TestMakeTimeFractionAndSign|TestMakeTimeReturnScale'`. Expect discarded fractions, NULL negative hours, and scale 0. + +- [ ] **Step 4: Implement return scale** + +```go +func makeTimeReturnType(parameters []types.Type) types.Type { + scale := parameters[2].Scale + if scale < 0 { + scale = 0 + } else if scale > 6 { + scale = 6 + } + return types.T_time.ToTypeWithScale(scale) +} +``` + +Use this helper for every MAKETIME overload. + +- [ ] **Step 5: Normalize float seconds** + +Integer extractors return `(value, 0, null)`. The float extractor becomes: + +```go +val, null := secondParam.GetValue(i) +if null || math.IsNaN(val) || math.IsInf(val, 0) || val < 0 || val >= 60 { + return 0, 0, true +} +total := int64(math.Round(val * float64(types.MicroSecsPerSec))) +return total / types.MicroSecsPerSec, uint32(total % types.MicroSecsPerSec), false +``` + +- [ ] **Step 6: Construct the signed normalized TIME** + +Validate `hour` in `[-838,838]`, `minute` in `[0,59]`, `second` in `[0,60]`, and `microsecond < types.MicroSecsPerSec`. Determine the sign before negating the hour. Call: + +```go +timeValue := types.TimeFromClock(isNegative, uint64(hour), uint8(minute), uint8(second), microsecond) +``` + +Inspect `ClockFormat` after normalization and append NULL when the result hour exceeds 838. Pass microseconds through the MakeTime row loop. + +- [ ] **Step 7: Run GREEN and adjacent regressions** + +Run `TestMakeTimeFractionAndSign|TestMakeTimeReturnScale`, then `TestMakeTime|TestTimeFormat|TestHour`. Expect PASS. + +- [ ] **Step 8: Commit** + +```bash +git add pkg/sql/plan/function/func_binary.go pkg/sql/plan/function/func_binary_test.go pkg/sql/plan/function/function_test.go pkg/sql/plan/function/list_builtIn.go +git commit -m "fix: preserve maketime fractional seconds" +``` + +### Task 4: Add SQL-Visible Regression Coverage + +**Files:** +- Modify: `test/distributed/cases/function/func_datetime_hour.test` +- Modify: `test/distributed/cases/function/func_datetime_hour.result` +- Modify: `test/distributed/cases/function/func_datetime_time_format.test` +- Modify: `test/distributed/cases/function/func_datetime_time_format.result` +- Modify: `test/distributed/cases/function/func_datetime_maketime.test` +- Modify: `test/distributed/cases/function/func_datetime_maketime.result` + +**Interfaces:** +- Produces: SQL-visible regressions for all five compatibility changes. + +- [ ] **Step 1: Add HOUR cases** + +Add positive and negative `272:59:59` casts. Both expected HOUR values are `272`. + +- [ ] **Step 2: Add TIME_FORMAT cases** + +Add `%T` for `123:45:06` expecting `123:45:06`, and `elapsed=%H:%i:%s` for `-123:45:06` expecting `-elapsed=123:45:06`. + +- [ ] **Step 3: Add and update MAKETIME cases** + +Add: + +```sql +SELECT MAKETIME(12, 34, 56.789012) AS fractional_seconds; +SELECT MAKETIME(-12, 34, 56.789012) AS negative_fractional; +SELECT MAKETIME(12, 59, 59.9999996) AS fractional_carry; +``` + +Expect `12:34:56.789012`, `-12:34:56.789012`, and `13:00:00.000000`. Update existing `MAKETIME(-1,15,30)` from NULL to `-01:15:30`; update `MAKETIME(12.7,15.8,30.9)` from `12:15:30` to `12:15:30.9` and revise its comment. + +- [ ] **Step 4: Inspect and commit** + +Run `git diff --check`, verify each statement has one result block, then commit the six files with `test: cover compatible time function semantics`. + +### Task 5: Full Verification and Independent Review + +**Files:** +- Inspect: complete diff versus `upstream/main`. +- Modify: only files implicated by a proven verification or review finding. + +- [ ] **Step 1: Format and inspect** + +Run gofmt on all changed Go files, `git diff --check`, `git diff --stat upstream/main...HEAD`, and `git status --short`. + +- [ ] **Step 2: Build, vet, and test** + +With the environment below, run `go build ./pkg/sql/plan/function`, `go vet ./pkg/sql/plan/function`, the complete function package test, and a dependent `pkg/sql/plan` test. Every command must exit 0. + +- [ ] **Step 3: Prove red-green** + +Retain the regression tests while temporarily reversing each implementation hunk. Run focused tests and record failures; restore the implementation and rerun to PASS. Confirm the restored diff is unchanged. + +- [ ] **Step 4: Review** + +Run `mo-self-review` against `upstream/main`, including functional closure and unhappy paths. Then dispatch the user-requested independent subagent with the issue, spec, plan, and full diff. Fix every concrete blocker and decision-log any rejected suggestion. + +- [ ] **Step 5: Fresh final verification** + +Repeat build, vet, complete package tests, dependent tests, `git diff --check`, and status inspection after all review fixes. + +## Verification Environment + +```bash +export CGO_CFLAGS="-I/Users/yanghaoyang/repo/matrixone/cgo -I/Users/yanghaoyang/repo/matrixone/thirdparties/install/include" +export CGO_LDFLAGS="-L/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib -lusearch_c" +export DYLD_LIBRARY_PATH="/Users/yanghaoyang/repo/matrixone/cgo:/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib" +export GO_TEST_LDFLAGS="-extldflags '-L/Users/yanghaoyang/repo/matrixone/cgo -lmo -L/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib -Wl,-rpath,/Users/yanghaoyang/repo/matrixone/cgo -Wl,-rpath,/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib'" +go test -ldflags="$GO_TEST_LDFLAGS" -count=1 -timeout 120s -v ./pkg/sql/plan/function +``` + From 85cb2cf0b61f889917e98cda1a1d2d9e66c5d414 Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 14:40:51 +0800 Subject: [PATCH 03/30] fix: preserve complete time hours --- pkg/sql/plan/function/func_unary.go | 5 ++--- pkg/sql/plan/function/func_unary_test.go | 13 ++++++++----- pkg/sql/plan/function/list_builtIn.go | 2 +- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/sql/plan/function/func_unary.go b/pkg/sql/plan/function/func_unary.go index 7ba49577bbbe8..b54bbb3153055 100644 --- a/pkg/sql/plan/function/func_unary.go +++ b/pkg/sql/plan/function/func_unary.go @@ -4545,10 +4545,9 @@ func DatetimeToHour(ivecs []*vector.Vector, result vector.FunctionResultWrapper, } func TimeToHour(ivecs []*vector.Vector, result vector.FunctionResultWrapper, proc *process.Process, length int, selectList *FunctionSelectList) error { - return opUnaryFixedToFixed[types.Time, uint8](ivecs, result, proc, length, func(v types.Time) uint8 { + return opUnaryFixedToFixed[types.Time, uint32](ivecs, result, proc, length, func(v types.Time) uint32 { hour, _, _, _, _ := v.ClockFormat() - // HOUR function returns 0-23, so we need to take modulo 24 - return uint8(hour % 24) + return uint32(hour) }, selectList) } diff --git a/pkg/sql/plan/function/func_unary_test.go b/pkg/sql/plan/function/func_unary_test.go index 35b2da0038bb1..45241fa949491 100644 --- a/pkg/sql/plan/function/func_unary_test.go +++ b/pkg/sql/plan/function/func_unary_test.go @@ -3859,6 +3859,9 @@ func initHourTestCase() []tcTemp { t1, _ := types.ParseTime("15:30:45", 6) t2, _ := types.ParseTime("00:00:00", 6) t3, _ := types.ParseTime("23:59:59", 6) + t4, _ := types.ParseTime("272:59:59", 6) + t5, _ := types.ParseTime("-272:59:59", 6) + t6 := types.TimeFromClock(false, types.MaxHourInTime, 59, 59, 0) return []tcTemp{ { @@ -3890,12 +3893,12 @@ func initHourTestCase() []tcTemp { typ: types.T_time, inputs: []FunctionTestInput{ NewFunctionTestInput(types.T_time.ToType(), - []types.Time{t1, t2, t3}, - []bool{false, false, false}), + []types.Time{t1, t2, t3, t4, t5, t6}, + []bool{false, false, false, false, false, false}), }, - expect: NewFunctionTestResult(types.T_uint8.ToType(), false, - []uint8{15, 0, 23}, - []bool{false, false, false}), + expect: NewFunctionTestResult(types.T_uint32.ToType(), false, + []uint32{15, 0, 23, 272, 272, uint32(types.MaxHourInTime)}, + []bool{false, false, false, false, false, false}), }, } } diff --git a/pkg/sql/plan/function/list_builtIn.go b/pkg/sql/plan/function/list_builtIn.go index 7eeda0690cd72..91c272dcb9f8b 100644 --- a/pkg/sql/plan/function/list_builtIn.go +++ b/pkg/sql/plan/function/list_builtIn.go @@ -9114,7 +9114,7 @@ var supportedDateAndTimeBuiltIns = []FuncNew{ overloadId: 2, args: []types.T{types.T_time}, retType: func(parameters []types.Type) types.Type { - return types.T_uint8.ToType() + return types.T_uint32.ToType() }, newOp: func() executeLogicOfOverload { return TimeToHour From 10ccc83889c348c59b614d2bc1b4e579ba259bee Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 14:42:31 +0800 Subject: [PATCH 04/30] fix: preserve time format hours and signs --- pkg/sql/plan/function/func_binary.go | 7 +++++-- pkg/sql/plan/function/func_binary_test.go | 20 +++++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index d16e7dcf5cd01..59eb52535a963 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -4043,7 +4043,10 @@ func TimeFormat(ivecs []*vector.Vector, result vector.FunctionResultWrapper, pro // timeFormat: Get the format string corresponding to the time according to format specifiers // Only supports time-related format specifiers: %H, %h, %I, %i, %k, %l, %S, %s, %f, %p, %r, %T func timeFormat(ctx context.Context, t types.Time, format string, buf *bytes.Buffer) error { - hour, minute, sec, msec, _ := t.ClockFormat() + hour, minute, sec, msec, isNeg := t.ClockFormat() + if isNeg { + buf.WriteByte('-') + } inPatternMatch := false for _, b := range format { if inPatternMatch { @@ -4112,7 +4115,7 @@ func makeTimeFormat(ctx context.Context, hour uint64, minute, sec uint8, msec ui case 'S', 's': FormatInt2BufByWidth(int(sec), 2, buf) case 'T': - fmt.Fprintf(buf, "%02d:%02d:%02d", hour%24, minute, sec) + fmt.Fprintf(buf, "%02d:%02d:%02d", hour, minute, sec) default: // For unsupported format specifiers, just write the character as-is // This matches MySQL behavior where non-time format specifiers are ignored diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index 5043f95639b40..e6148e3ec9da4 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9399,6 +9399,8 @@ func initTimeFormatTestCase() []tcTemp { t2, _ := types.ParseTime("00:00:00", 6) t3, _ := types.ParseTime("23:59:59.123456", 6) t4, _ := types.ParseTime("12:34:56.789012", 6) + t5, _ := types.ParseTime("123:45:06", 6) + t6, _ := types.ParseTime("-123:45:06", 6) return []tcTemp{ { @@ -9417,12 +9419,24 @@ func initTimeFormatTestCase() []tcTemp { info: "test time_format - %T", inputs: []FunctionTestInput{ NewFunctionTestInput(types.T_time.ToType(), - []types.Time{t1}, - []bool{false}), + []types.Time{t1, t5, t6}, + []bool{false, false, false}), NewFunctionTestConstInput(types.T_varchar.ToType(), []string{"%T"}, []bool{false}), }, expect: NewFunctionTestResult(types.T_varchar.ToType(), false, - []string{"15:30:45"}, + []string{"15:30:45", "123:45:06", "-123:45:06"}, + []bool{false, false, false}), + }, + { + info: "test time_format - negative time prefixes complete result", + inputs: []FunctionTestInput{ + NewFunctionTestInput(types.T_time.ToType(), + []types.Time{t6}, + []bool{false}), + NewFunctionTestConstInput(types.T_varchar.ToType(), []string{"elapsed=%H:%i:%s"}, []bool{false}), + }, + expect: NewFunctionTestResult(types.T_varchar.ToType(), false, + []string{"-elapsed=123:45:06"}, []bool{false}), }, { From d25fbadcef43299e9004be5ae729e4b4f354634e Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 14:49:34 +0800 Subject: [PATCH 05/30] fix: preserve maketime fractional seconds --- pkg/sql/plan/function/func_binary.go | 59 +++++++++++------------ pkg/sql/plan/function/func_binary_test.go | 31 ++++++++++++ pkg/sql/plan/function/function_test.go | 21 ++++++++ pkg/sql/plan/function/list_builtIn.go | 30 ++++++------ 4 files changed, 96 insertions(+), 45 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 59eb52535a963..0b746dfc723f6 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7274,28 +7274,23 @@ func MakeDateString( } // makeTimeFromInt64: Helper function to create Time from int64 values -func makeTimeFromInt64(hour, minute, second int64, rs *vector.FunctionResult[types.Time], i uint64) error { - // MySQL allows hour to be in range [0, 838] (TIME type range) - // minute and second should be in range [0, 59] - // If values are out of range, MySQL returns NULL - if hour < 0 || hour > 838 { +func makeTimeFromInt64(hour, minute, second int64, microsecond uint32, rs *vector.FunctionResult[types.Time], i uint64) error { + if hour < -838 || hour > 838 { return rs.Append(types.Time(0), true) } - if minute < 0 || minute > 59 || second < 0 || second > 59 { + if minute < 0 || minute > 59 || second < 0 || second > 60 || microsecond >= types.MicroSecsPerSec { return rs.Append(types.Time(0), true) } - // Create Time value using TimeFromClock - // hour can be up to 838, so we use uint64 for hour - timeValue := types.TimeFromClock(false, uint64(hour), uint8(minute), uint8(second), 0) - - // Validate the resulting time - h := timeValue.Hour() - if h < 0 { - h = -h + isNegative := hour < 0 + if isNegative { + hour = -hour } - if !types.ValidTime(uint64(h), 0, 0) { + timeValue := types.TimeFromClock(isNegative, uint64(hour), uint8(minute), uint8(second), microsecond) + + normalizedHour, _, _, _, _ := timeValue.ClockFormat() + if normalizedHour > 838 { return rs.Append(types.Time(0), true) } @@ -7314,7 +7309,7 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr // Create parameter wrappers based on types (these can be reused for all rows) var getHourValue func(uint64) (int64, bool) var getMinuteValue func(uint64) (int64, bool) - var getSecondValue func(uint64) (int64, bool) + var getSecondValue func(uint64) (int64, uint32, bool) // Setup hour parameter extractor switch hourType { @@ -7404,39 +7399,43 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr switch secondType { case types.T_int8: secondParam := vector.GenerateFunctionFixedTypeParameter[int8](ivecs[2]) - getSecondValue = func(i uint64) (int64, bool) { + getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return int64(val), null + return int64(val), 0, null } case types.T_int16: secondParam := vector.GenerateFunctionFixedTypeParameter[int16](ivecs[2]) - getSecondValue = func(i uint64) (int64, bool) { + getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return int64(val), null + return int64(val), 0, null } case types.T_int32: secondParam := vector.GenerateFunctionFixedTypeParameter[int32](ivecs[2]) - getSecondValue = func(i uint64) (int64, bool) { + getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return int64(val), null + return int64(val), 0, null } case types.T_int64: secondParam := vector.GenerateFunctionFixedTypeParameter[int64](ivecs[2]) - getSecondValue = func(i uint64) (int64, bool) { + getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return val, null + return val, 0, null } case types.T_uint8, types.T_uint16, types.T_uint32, types.T_uint64: secondParam := vector.GenerateFunctionFixedTypeParameter[uint64](ivecs[2]) - getSecondValue = func(i uint64) (int64, bool) { + getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return int64(val), null + return int64(val), 0, null } case types.T_float32, types.T_float64: secondParam := vector.GenerateFunctionFixedTypeParameter[float64](ivecs[2]) - getSecondValue = func(i uint64) (int64, bool) { + getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return int64(val), null // Truncate decimal part + if null || math.IsNaN(val) || math.IsInf(val, 0) || val < 0 || val >= 60 { + return 0, 0, true + } + total := int64(math.Round(val * float64(types.MicroSecsPerSec))) + return total / types.MicroSecsPerSec, uint32(total % types.MicroSecsPerSec), false } default: return moerr.NewInvalidArgNoCtx("MAKETIME second parameter", secondType) @@ -7446,7 +7445,7 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr for i := uint64(0); i < uint64(length); i++ { hourInt, null1 := getHourValue(i) minuteInt, null2 := getMinuteValue(i) - secondInt, null3 := getSecondValue(i) + secondInt, microsecond, null3 := getSecondValue(i) if null1 || null2 || null3 { if err := rs.Append(types.Time(0), true); err != nil { @@ -7455,7 +7454,7 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr continue } - if err := makeTimeFromInt64(hourInt, minuteInt, secondInt, rs, i); err != nil { + if err := makeTimeFromInt64(hourInt, minuteInt, secondInt, microsecond, rs, i); err != nil { return err } } diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index e6148e3ec9da4..d146ac12c9a63 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9501,6 +9501,37 @@ func TestTimeFormat(t *testing.T) { } } +func TestMakeTimeFractionAndSign(t *testing.T) { + proc := testutil.NewProcess(t) + floatWithMicrosecondScale := types.T_float64.ToTypeWithScale(6) + fcTC := NewFunctionTestCase(proc, + []FunctionTestInput{ + NewFunctionTestInput(floatWithMicrosecondScale, + []float64{12, -12, 12, 838, 12, 12}, + []bool{false, false, false, false, false, false}), + NewFunctionTestInput(floatWithMicrosecondScale, + []float64{34, 34, 59, 59, 34, 34}, + []bool{false, false, false, false, false, false}), + NewFunctionTestInput(floatWithMicrosecondScale, + []float64{56.789012, 56.789012, 59.9999996, 59.9999996, math.NaN(), math.Inf(1)}, + []bool{false, false, false, false, false, false}), + }, + NewFunctionTestResult(types.T_time.ToTypeWithScale(6), false, + []types.Time{ + types.TimeFromClock(false, 12, 34, 56, 789012), + types.TimeFromClock(true, 12, 34, 56, 789012), + types.TimeFromClock(false, 13, 0, 0, 0), + 0, + 0, + 0, + }, + []bool{false, false, false, true, true, true}), + MakeTime) + + s, info := fcTC.Run() + require.True(t, s, "MAKETIME fractional/sign case failed: %s", info) +} + // TestTimestampDiffDateString tests TIMESTAMPDIFF with DATE and string arguments // This tests the new overload that handles mixed DATE and string types func TestTimestampDiffDateString(t *testing.T) { diff --git a/pkg/sql/plan/function/function_test.go b/pkg/sql/plan/function/function_test.go index 137363baee5c3..4db532491253f 100644 --- a/pkg/sql/plan/function/function_test.go +++ b/pkg/sql/plan/function/function_test.go @@ -354,6 +354,27 @@ func Test_GetFunctionByName(t *testing.T) { } } +func TestMakeTimeReturnScale(t *testing.T) { + proc := testutil.NewProcess(t) + + integerResult, err := GetFunctionByName(proc.Ctx, "maketime", []types.Type{ + types.T_int64.ToType(), + types.T_int64.ToType(), + types.T_int64.ToType(), + }) + require.NoError(t, err) + require.Equal(t, types.T_time.ToType(), integerResult.retType) + + fractionalResult, err := GetFunctionByName(proc.Ctx, "maketime", []types.Type{ + types.T_int64.ToType(), + types.T_int64.ToType(), + types.New(types.T_decimal128, 20, 6), + }) + require.NoError(t, err) + require.True(t, fractionalResult.needCast) + require.Equal(t, types.T_time.ToTypeWithScale(6), fractionalResult.retType) +} + func TestGetFunctionByNameAESDecryptReturnsBlob(t *testing.T) { proc := testutil.NewProcess(t) tests := []struct { diff --git a/pkg/sql/plan/function/list_builtIn.go b/pkg/sql/plan/function/list_builtIn.go index 91c272dcb9f8b..bbeb081537f48 100644 --- a/pkg/sql/plan/function/list_builtIn.go +++ b/pkg/sql/plan/function/list_builtIn.go @@ -10737,6 +10737,16 @@ var supportedDateAndTimeBuiltIns = []FuncNew{ }, } +func makeTimeReturnType(parameters []types.Type) types.Type { + scale := parameters[2].Scale + if scale < 0 { + scale = 0 + } else if scale > 6 { + scale = 6 + } + return types.T_time.ToTypeWithScale(scale) +} + var supportedControlBuiltIns = []FuncNew{ // function `add_fault_point` { @@ -11192,9 +11202,7 @@ var supportedControlBuiltIns = []FuncNew{ { overloadId: 0, args: []types.T{types.T_int64, types.T_int64, types.T_int64}, - retType: func(parameters []types.Type) types.Type { - return types.T_time.ToType() - }, + retType: makeTimeReturnType, newOp: func() executeLogicOfOverload { return MakeTime }, @@ -11202,9 +11210,7 @@ var supportedControlBuiltIns = []FuncNew{ { overloadId: 1, args: []types.T{types.T_uint64, types.T_uint64, types.T_uint64}, - retType: func(parameters []types.Type) types.Type { - return types.T_time.ToType() - }, + retType: makeTimeReturnType, newOp: func() executeLogicOfOverload { return MakeTime }, @@ -11212,9 +11218,7 @@ var supportedControlBuiltIns = []FuncNew{ { overloadId: 2, args: []types.T{types.T_float64, types.T_float64, types.T_float64}, - retType: func(parameters []types.Type) types.Type { - return types.T_time.ToType() - }, + retType: makeTimeReturnType, newOp: func() executeLogicOfOverload { return MakeTime }, @@ -11222,9 +11226,7 @@ var supportedControlBuiltIns = []FuncNew{ { overloadId: 3, args: []types.T{types.T_int32, types.T_int32, types.T_int32}, - retType: func(parameters []types.Type) types.Type { - return types.T_time.ToType() - }, + retType: makeTimeReturnType, newOp: func() executeLogicOfOverload { return MakeTime }, @@ -11232,9 +11234,7 @@ var supportedControlBuiltIns = []FuncNew{ { overloadId: 4, args: []types.T{types.T_uint32, types.T_uint32, types.T_uint32}, - retType: func(parameters []types.Type) types.Type { - return types.T_time.ToType() - }, + retType: makeTimeReturnType, newOp: func() executeLogicOfOverload { return MakeTime }, From 5169d6452ea52cd274be8ad8878a02c20719fed2 Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 15:04:14 +0800 Subject: [PATCH 06/30] fix: reject overflowing maketime hours --- pkg/sql/plan/function/func_binary.go | 3 +++ pkg/sql/plan/function/func_binary_test.go | 26 +++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 0b746dfc723f6..d7e17af599302 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7341,6 +7341,9 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr hourParam := vector.GenerateFunctionFixedTypeParameter[uint64](ivecs[0]) getHourValue = func(i uint64) (int64, bool) { val, null := hourParam.GetValue(i) + if null || val > 838 { + return 0, true + } return int64(val), null } case types.T_float32, types.T_float64: diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index d146ac12c9a63..c0a1cc72310ab 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9532,6 +9532,32 @@ func TestMakeTimeFractionAndSign(t *testing.T) { require.True(t, s, "MAKETIME fractional/sign case failed: %s", info) } +func TestMakeTimeUnsignedHourOverflow(t *testing.T) { + proc := testutil.NewProcess(t) + fcTC := NewFunctionTestCase(proc, + []FunctionTestInput{ + NewFunctionTestInput(types.T_uint64.ToType(), + []uint64{838, math.MaxUint64}, + []bool{false, false}), + NewFunctionTestInput(types.T_uint64.ToType(), + []uint64{34, 34}, + []bool{false, false}), + NewFunctionTestInput(types.T_uint64.ToType(), + []uint64{56, 56}, + []bool{false, false}), + }, + NewFunctionTestResult(types.T_time.ToType(), false, + []types.Time{ + types.TimeFromClock(false, 838, 34, 56, 0), + 0, + }, + []bool{false, true}), + MakeTime) + + s, info := fcTC.Run() + require.True(t, s, "MAKETIME unsigned hour overflow case failed: %s", info) +} + // TestTimestampDiffDateString tests TIMESTAMPDIFF with DATE and string arguments // This tests the new overload that handles mixed DATE and string types func TestTimestampDiffDateString(t *testing.T) { From 9f95b7c87899732ce10118ebb31bb3229cb2af09 Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 15:09:51 +0800 Subject: [PATCH 07/30] test: cover compatible time function semantics --- .../cases/function/func_datetime_hour.result | 6 ++++++ .../cases/function/func_datetime_hour.test | 2 ++ .../cases/function/func_datetime_maketime.result | 13 +++++++++++-- .../cases/function/func_datetime_maketime.test | 7 +++++-- .../cases/function/func_datetime_time_format.result | 6 ++++++ .../cases/function/func_datetime_time_format.test | 2 ++ 6 files changed, 32 insertions(+), 4 deletions(-) diff --git a/test/distributed/cases/function/func_datetime_hour.result b/test/distributed/cases/function/func_datetime_hour.result index 487baff41f9ea..d59d208c3ec4c 100644 --- a/test/distributed/cases/function/func_datetime_hour.result +++ b/test/distributed/cases/function/func_datetime_hour.result @@ -15,6 +15,12 @@ midnight SELECT HOUR(CAST('23:59:59' AS TIME)) AS end_of_day; end_of_day 23 +SELECT HOUR(CAST('272:59:59' AS TIME)) AS extended_positive; +extended_positive +272 +SELECT HOUR(CAST('-272:59:59' AS TIME)) AS extended_negative; +extended_negative +272 SELECT HOUR(CAST('2024-12-20 10:20:30' AS DATETIME)) AS datetime_hour; datetime_hour 10 diff --git a/test/distributed/cases/function/func_datetime_hour.test b/test/distributed/cases/function/func_datetime_hour.test index 50d612d77e730..43b575079e80b 100644 --- a/test/distributed/cases/function/func_datetime_hour.test +++ b/test/distributed/cases/function/func_datetime_hour.test @@ -9,6 +9,8 @@ SELECT HOUR(NOW()) AS result3; SELECT HOUR(CAST('15:30:45' AS TIME)) AS time_cast; SELECT HOUR(CAST('00:00:00' AS TIME)) AS midnight; SELECT HOUR(CAST('23:59:59' AS TIME)) AS end_of_day; +SELECT HOUR(CAST('272:59:59' AS TIME)) AS extended_positive; +SELECT HOUR(CAST('-272:59:59' AS TIME)) AS extended_negative; # Additional test cases with DATETIME type SELECT HOUR(CAST('2024-12-20 10:20:30' AS DATETIME)) AS datetime_hour; diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index 5beaddbaf6a12..d8ed6a6c27290 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -27,7 +27,7 @@ time8 100:00:00 SELECT MAKETIME(-1, 15, 30) AS invalid_hour; invalid_hour -null +-01:15:30 SELECT MAKETIME(12, 60, 30) AS invalid_minute; invalid_minute null @@ -51,7 +51,16 @@ null_all null SELECT MAKETIME(12.7, 15.8, 30.9) AS float_values; float_values -12:15:30 +12:15:30.9 +SELECT MAKETIME(12, 34, 56.789012) AS fractional_seconds; +fractional_seconds +12:34:56.789012 +SELECT MAKETIME(-12, 34, 56.789012) AS negative_fractional; +negative_fractional +-12:34:56.789012 +SELECT MAKETIME(12, 59, 59.9999996) AS fractional_carry; +fractional_carry +13:00:00.000000 CREATE TABLE t1(h INT, m INT, s INT); INSERT INTO t1 VALUES (12, 15, 30), (0, 0, 0), (23, 59, 59); SELECT MAKETIME(h, m, s) AS time_value FROM t1; diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index 1a752353f892e..42c8d530e2571 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -16,7 +16,7 @@ SELECT MAKETIME(12, 15, 0) AS time6; SELECT MAKETIME(838, 59, 59) AS time7; SELECT MAKETIME(100, 0, 0) AS time8; -# Invalid values (should return NULL) +# Negative hours are valid; other invalid values return NULL SELECT MAKETIME(-1, 15, 30) AS invalid_hour; SELECT MAKETIME(12, 60, 30) AS invalid_minute; SELECT MAKETIME(12, 15, 60) AS invalid_second; @@ -28,8 +28,11 @@ SELECT MAKETIME(12, NULL, 30) AS null_minute; SELECT MAKETIME(12, 15, NULL) AS null_second; SELECT MAKETIME(NULL, NULL, NULL) AS null_all; -# Float values (should truncate) +# Float hour and minute values truncate; fractional seconds are preserved SELECT MAKETIME(12.7, 15.8, 30.9) AS float_values; +SELECT MAKETIME(12, 34, 56.789012) AS fractional_seconds; +SELECT MAKETIME(-12, 34, 56.789012) AS negative_fractional; +SELECT MAKETIME(12, 59, 59.9999996) AS fractional_carry; # Table usage CREATE TABLE t1(h INT, m INT, s INT); diff --git a/test/distributed/cases/function/func_datetime_time_format.result b/test/distributed/cases/function/func_datetime_time_format.result index a28105870aaff..332bc6baa38aa 100644 --- a/test/distributed/cases/function/func_datetime_time_format.result +++ b/test/distributed/cases/function/func_datetime_time_format.result @@ -7,6 +7,12 @@ basic_format SELECT TIME_FORMAT('15:30:45', '%T') AS t_format; t_format 15:30:45 +SELECT TIME_FORMAT('123:45:06', '%T') AS extended_t_format; +extended_t_format +123:45:06 +SELECT TIME_FORMAT('-123:45:06', 'elapsed=%H:%i:%s') AS negative_with_text; +negative_with_text +-elapsed=123:45:06 SELECT TIME_FORMAT('00:00:00', '%H:%i:%s') AS zero_time; zero_time 00:00:00 diff --git a/test/distributed/cases/function/func_datetime_time_format.test b/test/distributed/cases/function/func_datetime_time_format.test index b80e9ce5c63d5..a45f894d2e59e 100644 --- a/test/distributed/cases/function/func_datetime_time_format.test +++ b/test/distributed/cases/function/func_datetime_time_format.test @@ -5,6 +5,8 @@ SELECT TIME_FORMAT('15:30:45', '%H:%i:%s') AS result1; # Basic time formats SELECT TIME_FORMAT('15:30:45', '%H:%i:%s') AS basic_format; SELECT TIME_FORMAT('15:30:45', '%T') AS t_format; +SELECT TIME_FORMAT('123:45:06', '%T') AS extended_t_format; +SELECT TIME_FORMAT('-123:45:06', 'elapsed=%H:%i:%s') AS negative_with_text; SELECT TIME_FORMAT('00:00:00', '%H:%i:%s') AS zero_time; SELECT TIME_FORMAT('23:59:59', '%H:%i:%s') AS max_time; From 6af98ceed3c946d895cc37c41ac29b8cce7d475b Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 15:15:26 +0800 Subject: [PATCH 08/30] docs: fix time function plan formatting --- docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md | 1 - .../specs/2026-07-16-issue-24784-time-functions-design.md | 1 - 2 files changed, 2 deletions(-) diff --git a/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md b/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md index 62c6d87f127f1..adab20733b928 100644 --- a/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md +++ b/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md @@ -282,4 +282,3 @@ export DYLD_LIBRARY_PATH="/Users/yanghaoyang/repo/matrixone/cgo:/Users/yanghaoya export GO_TEST_LDFLAGS="-extldflags '-L/Users/yanghaoyang/repo/matrixone/cgo -lmo -L/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib -Wl,-rpath,/Users/yanghaoyang/repo/matrixone/cgo -Wl,-rpath,/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib'" go test -ldflags="$GO_TEST_LDFLAGS" -count=1 -timeout 120s -v ./pkg/sql/plan/function ``` - diff --git a/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md b/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md index 1c43495fa0c99..f0b9831c417d5 100644 --- a/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md +++ b/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md @@ -129,4 +129,3 @@ Implementation will follow red-green-refactor: This change does not redesign MatrixOne's `TIME` representation, add new decimal execution kernels, change time-zone behavior, or implement MySQL warning and saturation behavior for out-of-range `MAKETIME` values. - From bb34a3425a587e6a756b8530ec932f905902af99 Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 15:19:41 +0800 Subject: [PATCH 09/30] fix: reject raw maketime second carry --- pkg/sql/plan/function/func_binary.go | 17 +++++++--- pkg/sql/plan/function/func_binary_test.go | 38 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index d7e17af599302..19c9b0301e891 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7273,6 +7273,13 @@ func MakeDateString( return nil } +func makeTimeIntegerSecond(value int64, null bool) (int64, uint32, bool) { + if null || value < 0 || value >= 60 { + return 0, 0, true + } + return value, 0, false +} + // makeTimeFromInt64: Helper function to create Time from int64 values func makeTimeFromInt64(hour, minute, second int64, microsecond uint32, rs *vector.FunctionResult[types.Time], i uint64) error { if hour < -838 || hour > 838 { @@ -7404,31 +7411,31 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr secondParam := vector.GenerateFunctionFixedTypeParameter[int8](ivecs[2]) getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return int64(val), 0, null + return makeTimeIntegerSecond(int64(val), null) } case types.T_int16: secondParam := vector.GenerateFunctionFixedTypeParameter[int16](ivecs[2]) getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return int64(val), 0, null + return makeTimeIntegerSecond(int64(val), null) } case types.T_int32: secondParam := vector.GenerateFunctionFixedTypeParameter[int32](ivecs[2]) getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return int64(val), 0, null + return makeTimeIntegerSecond(int64(val), null) } case types.T_int64: secondParam := vector.GenerateFunctionFixedTypeParameter[int64](ivecs[2]) getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return val, 0, null + return makeTimeIntegerSecond(val, null) } case types.T_uint8, types.T_uint16, types.T_uint32, types.T_uint64: secondParam := vector.GenerateFunctionFixedTypeParameter[uint64](ivecs[2]) getSecondValue = func(i uint64) (int64, uint32, bool) { val, null := secondParam.GetValue(i) - return int64(val), 0, null + return makeTimeIntegerSecond(int64(val), null) } case types.T_float32, types.T_float64: secondParam := vector.GenerateFunctionFixedTypeParameter[float64](ivecs[2]) diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index c0a1cc72310ab..608f606f1d997 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9558,6 +9558,44 @@ func TestMakeTimeUnsignedHourOverflow(t *testing.T) { require.True(t, s, "MAKETIME unsigned hour overflow case failed: %s", info) } +func TestMakeTimeIntegerSecondRange(t *testing.T) { + proc := testutil.NewProcess(t) + expected := NewFunctionTestResult(types.T_time.ToType(), false, + []types.Time{ + types.TimeFromClock(false, 12, 34, 59, 0), + 0, + }, + []bool{false, true}) + + t.Run("signed", func(t *testing.T) { + fcTC := NewFunctionTestCase(proc, + []FunctionTestInput{ + NewFunctionTestInput(types.T_int64.ToType(), []int64{12, 12}, []bool{false, false}), + NewFunctionTestInput(types.T_int64.ToType(), []int64{34, 34}, []bool{false, false}), + NewFunctionTestInput(types.T_int64.ToType(), []int64{59, 60}, []bool{false, false}), + }, + expected, + MakeTime) + + s, info := fcTC.Run() + require.True(t, s, "MAKETIME signed integer second range failed: %s", info) + }) + + t.Run("unsigned", func(t *testing.T) { + fcTC := NewFunctionTestCase(proc, + []FunctionTestInput{ + NewFunctionTestInput(types.T_uint64.ToType(), []uint64{12, 12}, []bool{false, false}), + NewFunctionTestInput(types.T_uint64.ToType(), []uint64{34, 34}, []bool{false, false}), + NewFunctionTestInput(types.T_uint64.ToType(), []uint64{59, 60}, []bool{false, false}), + }, + expected, + MakeTime) + + s, info := fcTC.Run() + require.True(t, s, "MAKETIME unsigned integer second range failed: %s", info) + }) +} + // TestTimestampDiffDateString tests TIMESTAMPDIFF with DATE and string arguments // This tests the new overload that handles mixed DATE and string types func TestTimestampDiffDateString(t *testing.T) { From e095ed6efa25ad2ee7147b1ad113d543d40eeef7 Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 15:35:42 +0800 Subject: [PATCH 10/30] test: preserve decimal maketime scale in expected output --- test/distributed/cases/function/func_datetime_maketime.result | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index d8ed6a6c27290..07e423911b147 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -78,7 +78,7 @@ h m s DROP TABLE t1; SELECT MAKETIME(12.0, 15.0, 30.0) AS float_input; float_input -12:15:30 +12:15:30.0 SELECT MAKETIME(CAST(12 AS UNSIGNED), CAST(15 AS UNSIGNED), CAST(30 AS UNSIGNED)) AS unsigned_input; unsigned_input 12:15:30 From 17691be2f3b247d1710f653e06dd472823d6be92 Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 15:39:12 +0800 Subject: [PATCH 11/30] chore: exclude internal planning notes from change --- .../2026-07-16-issue-24784-time-functions.md | 284 ------------------ ...07-16-issue-24784-time-functions-design.md | 131 -------- 2 files changed, 415 deletions(-) delete mode 100644 docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md delete mode 100644 docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md diff --git a/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md b/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md deleted file mode 100644 index adab20733b928..0000000000000 --- a/docs/superpowers/plans/2026-07-16-issue-24784-time-functions.md +++ /dev/null @@ -1,284 +0,0 @@ -# Issue #24784 TIME Functions Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make `HOUR(TIME)`, `TIME_FORMAT`, and `MAKETIME` preserve large hours, signs, fractional seconds, and SQL-visible precision. - -**Architecture:** Keep the fix in the existing SQL function registry and vectorized execution functions. Widen only the TIME overload of HOUR, centralize sign handling in `timeFormat`, and normalize MAKETIME seconds to whole seconds plus microseconds before constructing `types.Time`. - -**Tech Stack:** Go, MatrixOne vectorized SQL functions, `types.Time`, `testify`, distributed SQL test files. - -## Global Constraints - -- Work from `upstream/main` in the isolated `codex/fix-24784-time-functions` worktree. -- HOUR must cover every value through `types.MaxHourInTime` and ignore the TIME sign. -- TIME_FORMAT must emit one leading minus sign for negative TIME values. -- MAKETIME accepts hours in `[-838,838]`, minutes in `[0,59]`, and seconds in `[0,60)`. -- Round fractions to at most six microsecond digits and normalize carries. -- Invalid or out-of-range MAKETIME rows append NULL; warnings and saturation are out of scope. -- Add no dependencies and do not restructure the existing large function files. - ---- - -### Task 1: Preserve Complete HOUR(TIME) Values - -**Files:** -- Modify: `pkg/sql/plan/function/func_unary_test.go` -- Modify: `pkg/sql/plan/function/func_unary.go` -- Modify: `pkg/sql/plan/function/list_builtIn.go` - -**Interfaces:** -- Consumes: `types.Time.ClockFormat()`. -- Produces: TIME overload results as `uint32`; HOUR overload 2 returns `types.T_uint32`. - -- [ ] **Step 1: Write the failing test** - -Extend the TIME case in `initHourTestCase` with positive and negative 272-hour values and `types.MaxHourInTime`. Its expected result is: - -```go -NewFunctionTestResult(types.T_uint32.ToType(), false, - []uint32{15, 0, 23, 272, 272, uint32(types.MaxHourInTime)}, - []bool{false, false, false, false, false, false}) -``` - -- [ ] **Step 2: Run RED** - -Run the verification test command with `-run '^TestHour$'`. Expect a type mismatch and the 272-hour value folded to 16. - -- [ ] **Step 3: Implement the minimal fix** - -```go -func TimeToHour(ivecs []*vector.Vector, result vector.FunctionResultWrapper, proc *process.Process, length int, selectList *FunctionSelectList) error { - return opUnaryFixedToFixed[types.Time, uint32](ivecs, result, proc, length, func(v types.Time) uint32 { - hour, _, _, _, _ := v.ClockFormat() - return uint32(hour) - }, selectList) -} -``` - -Change only HOUR overload 2 in `list_builtIn.go` to `types.T_uint32.ToType()`. - -- [ ] **Step 4: Run GREEN** - -Run `TestHour` again and expect PASS. - -- [ ] **Step 5: Commit** - -```bash -git add pkg/sql/plan/function/func_unary.go pkg/sql/plan/function/func_unary_test.go pkg/sql/plan/function/list_builtIn.go -git commit -m "fix: preserve complete time hours" -``` - -### Task 2: Preserve TIME_FORMAT Hours and Signs - -**Files:** -- Modify: `pkg/sql/plan/function/func_binary_test.go` -- Modify: `pkg/sql/plan/function/func_binary.go` - -**Interfaces:** -- Consumes: `ClockFormat` including `isNeg`. -- Produces: one leading sign and a full-hour `%T` rendering. - -- [ ] **Step 1: Write the failing tests** - -Extend the `%T` case with `123:45:06` and `-123:45:06`, expecting: - -```go -[]string{"15:30:45", "123:45:06", "-123:45:06"} -``` - -Add a negative value with format `elapsed=%H:%i:%s`, expecting `-elapsed=123:45:06`. - -- [ ] **Step 2: Run RED** - -Run the verification test command with `-run '^TestTimeFormat$'`. Expect `%T` to produce `03:45:06` and the sign to be absent. - -- [ ] **Step 3: Implement sign and hour handling** - -At the start of `timeFormat`: - -```go -hour, minute, sec, msec, isNeg := t.ClockFormat() -if isNeg { - buf.WriteByte('-') -} -``` - -Change `%T` to: - -```go -fmt.Fprintf(buf, "%02d:%02d:%02d", hour, minute, sec) -``` - -- [ ] **Step 4: Run GREEN** - -Run `TestTimeFormat` again and expect PASS. - -- [ ] **Step 5: Commit** - -```bash -git add pkg/sql/plan/function/func_binary.go pkg/sql/plan/function/func_binary_test.go -git commit -m "fix: preserve time format hours and signs" -``` - -### Task 3: Preserve MAKETIME Fractions, Scale, Carries, and Negative Hours - -**Files:** -- Modify: `pkg/sql/plan/function/func_binary_test.go` -- Modify: `pkg/sql/plan/function/function_test.go` -- Modify: `pkg/sql/plan/function/func_binary.go` -- Modify: `pkg/sql/plan/function/list_builtIn.go` - -**Interfaces:** -- Produces: `makeTimeReturnType(parameters []types.Type) types.Type`. -- Produces: `makeTimeFromInt64(hour, minute, second int64, microsecond uint32, rs *vector.FunctionResult[types.Time], i uint64) error`. -- Produces: second extractors returning `(int64, uint32, bool)`. - -- [ ] **Step 1: Write failing execution tests** - -Add `TestMakeTimeFractionAndSign` with float64 scale-6 vectors for these rows: - -```go -hours := []float64{12, -12, 12, 838, 12, 12} -minutes := []float64{34, 34, 59, 59, 34, 34} -seconds := []float64{56.789012, 56.789012, 59.9999996, 59.9999996, math.NaN(), math.Inf(1)} -expected := []types.Time{ - types.TimeFromClock(false, 12, 34, 56, 789012), - types.TimeFromClock(true, 12, 34, 56, 789012), - types.TimeFromClock(false, 13, 0, 0, 0), 0, 0, 0, -} -nulls := []bool{false, false, false, true, true, true} -``` - -Use a TIME(6) expected result. - -- [ ] **Step 2: Write failing return-scale tests** - -Add `TestMakeTimeReturnScale` in `function_test.go`. Assert all-int64 arguments return TIME(0). Resolve arguments `(int64, int64, decimal128(20,6))`; assert an implicit cast is requested and the result is TIME(6). - -- [ ] **Step 3: Run RED** - -Run with `-run 'TestMakeTimeFractionAndSign|TestMakeTimeReturnScale'`. Expect discarded fractions, NULL negative hours, and scale 0. - -- [ ] **Step 4: Implement return scale** - -```go -func makeTimeReturnType(parameters []types.Type) types.Type { - scale := parameters[2].Scale - if scale < 0 { - scale = 0 - } else if scale > 6 { - scale = 6 - } - return types.T_time.ToTypeWithScale(scale) -} -``` - -Use this helper for every MAKETIME overload. - -- [ ] **Step 5: Normalize float seconds** - -Integer extractors return `(value, 0, null)`. The float extractor becomes: - -```go -val, null := secondParam.GetValue(i) -if null || math.IsNaN(val) || math.IsInf(val, 0) || val < 0 || val >= 60 { - return 0, 0, true -} -total := int64(math.Round(val * float64(types.MicroSecsPerSec))) -return total / types.MicroSecsPerSec, uint32(total % types.MicroSecsPerSec), false -``` - -- [ ] **Step 6: Construct the signed normalized TIME** - -Validate `hour` in `[-838,838]`, `minute` in `[0,59]`, `second` in `[0,60]`, and `microsecond < types.MicroSecsPerSec`. Determine the sign before negating the hour. Call: - -```go -timeValue := types.TimeFromClock(isNegative, uint64(hour), uint8(minute), uint8(second), microsecond) -``` - -Inspect `ClockFormat` after normalization and append NULL when the result hour exceeds 838. Pass microseconds through the MakeTime row loop. - -- [ ] **Step 7: Run GREEN and adjacent regressions** - -Run `TestMakeTimeFractionAndSign|TestMakeTimeReturnScale`, then `TestMakeTime|TestTimeFormat|TestHour`. Expect PASS. - -- [ ] **Step 8: Commit** - -```bash -git add pkg/sql/plan/function/func_binary.go pkg/sql/plan/function/func_binary_test.go pkg/sql/plan/function/function_test.go pkg/sql/plan/function/list_builtIn.go -git commit -m "fix: preserve maketime fractional seconds" -``` - -### Task 4: Add SQL-Visible Regression Coverage - -**Files:** -- Modify: `test/distributed/cases/function/func_datetime_hour.test` -- Modify: `test/distributed/cases/function/func_datetime_hour.result` -- Modify: `test/distributed/cases/function/func_datetime_time_format.test` -- Modify: `test/distributed/cases/function/func_datetime_time_format.result` -- Modify: `test/distributed/cases/function/func_datetime_maketime.test` -- Modify: `test/distributed/cases/function/func_datetime_maketime.result` - -**Interfaces:** -- Produces: SQL-visible regressions for all five compatibility changes. - -- [ ] **Step 1: Add HOUR cases** - -Add positive and negative `272:59:59` casts. Both expected HOUR values are `272`. - -- [ ] **Step 2: Add TIME_FORMAT cases** - -Add `%T` for `123:45:06` expecting `123:45:06`, and `elapsed=%H:%i:%s` for `-123:45:06` expecting `-elapsed=123:45:06`. - -- [ ] **Step 3: Add and update MAKETIME cases** - -Add: - -```sql -SELECT MAKETIME(12, 34, 56.789012) AS fractional_seconds; -SELECT MAKETIME(-12, 34, 56.789012) AS negative_fractional; -SELECT MAKETIME(12, 59, 59.9999996) AS fractional_carry; -``` - -Expect `12:34:56.789012`, `-12:34:56.789012`, and `13:00:00.000000`. Update existing `MAKETIME(-1,15,30)` from NULL to `-01:15:30`; update `MAKETIME(12.7,15.8,30.9)` from `12:15:30` to `12:15:30.9` and revise its comment. - -- [ ] **Step 4: Inspect and commit** - -Run `git diff --check`, verify each statement has one result block, then commit the six files with `test: cover compatible time function semantics`. - -### Task 5: Full Verification and Independent Review - -**Files:** -- Inspect: complete diff versus `upstream/main`. -- Modify: only files implicated by a proven verification or review finding. - -- [ ] **Step 1: Format and inspect** - -Run gofmt on all changed Go files, `git diff --check`, `git diff --stat upstream/main...HEAD`, and `git status --short`. - -- [ ] **Step 2: Build, vet, and test** - -With the environment below, run `go build ./pkg/sql/plan/function`, `go vet ./pkg/sql/plan/function`, the complete function package test, and a dependent `pkg/sql/plan` test. Every command must exit 0. - -- [ ] **Step 3: Prove red-green** - -Retain the regression tests while temporarily reversing each implementation hunk. Run focused tests and record failures; restore the implementation and rerun to PASS. Confirm the restored diff is unchanged. - -- [ ] **Step 4: Review** - -Run `mo-self-review` against `upstream/main`, including functional closure and unhappy paths. Then dispatch the user-requested independent subagent with the issue, spec, plan, and full diff. Fix every concrete blocker and decision-log any rejected suggestion. - -- [ ] **Step 5: Fresh final verification** - -Repeat build, vet, complete package tests, dependent tests, `git diff --check`, and status inspection after all review fixes. - -## Verification Environment - -```bash -export CGO_CFLAGS="-I/Users/yanghaoyang/repo/matrixone/cgo -I/Users/yanghaoyang/repo/matrixone/thirdparties/install/include" -export CGO_LDFLAGS="-L/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib -lusearch_c" -export DYLD_LIBRARY_PATH="/Users/yanghaoyang/repo/matrixone/cgo:/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib" -export GO_TEST_LDFLAGS="-extldflags '-L/Users/yanghaoyang/repo/matrixone/cgo -lmo -L/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib -Wl,-rpath,/Users/yanghaoyang/repo/matrixone/cgo -Wl,-rpath,/Users/yanghaoyang/repo/matrixone/thirdparties/install/lib'" -go test -ldflags="$GO_TEST_LDFLAGS" -count=1 -timeout 120s -v ./pkg/sql/plan/function -``` diff --git a/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md b/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md deleted file mode 100644 index f0b9831c417d5..0000000000000 --- a/docs/superpowers/specs/2026-07-16-issue-24784-time-functions-design.md +++ /dev/null @@ -1,131 +0,0 @@ -# Issue #24784: MySQL-Compatible TIME Function Semantics - -## Context - -Issue #24784 reports three incorrect behaviors in the SQL function layer: - -- `HOUR(TIME)` folds hours with `% 24`. -- `TIME_FORMAT(TIME, "%T")` folds hours with `% 24`. -- `MAKETIME(hour, minute, second)` discards fractional seconds. - -Investigation also found two directly related compatibility gaps: `TIME_FORMAT` -drops the sign of negative `TIME` values, and `MAKETIME` rejects negative hours -that MySQL accepts. This change fixes all five behaviors on `main`. - -## Compatibility Contract - -The implementation will follow these rules: - -1. `HOUR(TIME)` returns the absolute hour component without folding it into a - 24-hour clock. This matches MySQL, which ignores the sign when extracting the - hour. -2. The `TIME` overload of `HOUR` returns `uint32`. MatrixOne's largest internal - `TIME` hour, `types.MaxHourInTime` (`2,562,047,787`), fits in `uint32`, so the - return type cannot truncate any valid MatrixOne `TIME` value. -3. `TIME_FORMAT` prefixes one minus sign for a negative `TIME`. The sign applies - to the complete formatted result, matching MySQL's formatting flow. -4. `%H`, `%k`, and `%T` preserve the complete hour component. Twelve-hour - specifiers (`%h`, `%I`, `%l`, `%p`, and `%r`) continue to use the hour modulo - 24 because they describe a time-of-day representation. -5. `MAKETIME` accepts hours from `-838` through `838`, minutes from `0` through - `59`, and seconds from `0` up to but not including `60`. Negative hours create - a negative `TIME`. -6. Fractional seconds are rounded to MatrixOne's six-digit microsecond storage - precision. A rounded value that reaches one complete second carries into the - next minute or hour. -7. The result `TIME` scale is inherited from the second argument's numeric scale - and clamped to `0..6`. Integer seconds therefore produce scale 0, while a - value such as `56.789012` produces scale 6. -8. `NaN`, positive or negative infinity, negative seconds, invalid minutes, and - values whose normalized result exceeds `838:59:59.999999` return `NULL`. -9. Existing out-of-range policy remains unchanged: this work does not add - MySQL-style warning emission or saturation for results beyond the supported - `MAKETIME` range. - -## Design - -### HOUR(TIME) - -`TimeToHour` will use `ClockFormat` and return its hour as `uint32`, without -`% 24`. The function registry will declare only the `TIME` overload as -`uint32`; timestamp and datetime overloads remain `uint8` because their hour is -always in `0..23`. - -The sign returned by `ClockFormat` is intentionally ignored. This is a -documented compatibility choice, not an omission. - -### TIME_FORMAT - -`timeFormat` will retain the sign returned by `ClockFormat` and write `-` to the -output buffer before processing the format string. This keeps sign handling in -one place and avoids duplicating it across individual specifiers. - -The `%T` branch in `makeTimeFormat` will format the complete hour directly. -Existing `%H` and `%k` behavior already preserves the complete hour and will not -change. Twelve-hour branches will remain unchanged. - -### MAKETIME Fraction Conversion - -The second-argument extractor will return a normalized representation containing -whole seconds and microseconds. Integer inputs produce zero microseconds. Float -inputs will: - -1. reject non-finite values and values outside `[0, 60)`; -2. round `value * types.MicroSecsPerSec` to the nearest integer microsecond; -3. split the result into whole seconds and microseconds. - -Splitting after rounding naturally represents a carry as 60 whole seconds and -zero microseconds. Construction will use total duration semantics through -`types.TimeFromClock`, which normalizes the carry. The normalized value is then -validated against the supported `MAKETIME` range before it is appended. - -The hour helper will determine the sign, take the absolute hour after range -validation, and pass both to `types.TimeFromClock`. Range validation occurs -before absolute-value conversion, so there is no `math.MinInt64` overflow path. - -### Result Precision - -Each `MAKETIME` overload will use a shared return-type helper. It will copy the -third parameter's scale, clamp negative values to 0 and values above 6 to 6, and -return `types.New(types.T_time, 0, scale)`. - -The planner preserves source numeric scale when it inserts an implicit cast to -`float64`, so decimal literals such as `56.789012` retain scale 6 through overload -resolution. This allows frontend serialization to call `Time.String2(6)` and -display the stored fraction. - -## Error and Boundary Handling - -- NULL propagation remains handled by the strict function contract and the - existing vector null checks. -- Invalid rows append a NULL result without aborting evaluation of the batch. -- Floating-point validation happens before conversion to integers, preventing - implementation-defined-looking results for `NaN` and infinity. -- A fractional carry is normalized before final range validation. -- No new allocations, goroutines, locks, waits, or external resources are - introduced. - -## Testing Strategy - -Implementation will follow red-green-refactor: - -1. Extend `TestHour` with hours above 23, a negative `TIME`, and - `types.MaxHourInTime`; assert the `uint32` result type. -2. Extend `TestTimeFormat` with a large-hour `%T` case and negative formats that - verify a single leading sign. -3. Add focused `MAKETIME` unit cases for six-digit fractions, negative hours, - fractional carry, NULLs, invalid numeric values, and the upper boundary. -4. Add function-resolution tests proving that integer seconds return `TIME(0)` - and a scale-6 numeric second returns `TIME(6)` after implicit casting. -5. Update distributed `MAKETIME`, `TIME_FORMAT`, and HOUR cases and expected - results to exercise SQL-visible formatting. -6. Prove the regression tests fail without the implementation and pass with it. -7. Run formatting, package build, `go vet`, package tests with `-count=1`, a - dependent-package regression test, and relevant race tests if the test - environment supports them. - -## Scope Boundaries - -This change does not redesign MatrixOne's `TIME` representation, add new decimal -execution kernels, change time-zone behavior, or implement MySQL warning and -saturation behavior for out-of-range `MAKETIME` values. From 2a3913dbd3035bd849f8240b61f9a618c6c4e1ad Mon Sep 17 00:00:00 2001 From: ULookup Date: Thu, 16 Jul 2026 17:25:06 +0800 Subject: [PATCH 12/30] test: stabilize maketime BVT output --- .../cases/function/func_datetime_maketime.result | 12 ++++++------ .../cases/function/func_datetime_maketime.test | 13 ++++++------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index 07e423911b147..8d07f31630d13 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -25,7 +25,7 @@ time7 SELECT MAKETIME(100, 0, 0) AS time8; time8 100:00:00 -SELECT MAKETIME(-1, 15, 30) AS invalid_hour; +SELECT CAST(MAKETIME(-1, 15, 30) AS VARCHAR) AS invalid_hour; invalid_hour -01:15:30 SELECT MAKETIME(12, 60, 30) AS invalid_minute; @@ -49,16 +49,16 @@ null SELECT MAKETIME(NULL, NULL, NULL) AS null_all; null_all null -SELECT MAKETIME(12.7, 15.8, 30.9) AS float_values; +SELECT CAST(MAKETIME(12.7, 15.8, 30.9) AS VARCHAR) AS float_values; float_values 12:15:30.9 -SELECT MAKETIME(12, 34, 56.789012) AS fractional_seconds; +SELECT CAST(MAKETIME(12, 34, 56.789012) AS VARCHAR) AS fractional_seconds; fractional_seconds 12:34:56.789012 -SELECT MAKETIME(-12, 34, 56.789012) AS negative_fractional; +SELECT CAST(MAKETIME(-12, 34, 56.789012) AS VARCHAR) AS negative_fractional; negative_fractional -12:34:56.789012 -SELECT MAKETIME(12, 59, 59.9999996) AS fractional_carry; +SELECT CAST(MAKETIME(12, 59, 59.9999996) AS VARCHAR) AS fractional_carry; fractional_carry 13:00:00.000000 CREATE TABLE t1(h INT, m INT, s INT); @@ -76,7 +76,7 @@ h m s 12 15 30 20 0 0 DROP TABLE t1; -SELECT MAKETIME(12.0, 15.0, 30.0) AS float_input; +SELECT CAST(MAKETIME(12.0, 15.0, 30.0) AS VARCHAR) AS float_input; float_input 12:15:30.0 SELECT MAKETIME(CAST(12 AS UNSIGNED), CAST(15 AS UNSIGNED), CAST(30 AS UNSIGNED)) AS unsigned_input; diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index 42c8d530e2571..1f4dd7326ed4c 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -17,7 +17,7 @@ SELECT MAKETIME(838, 59, 59) AS time7; SELECT MAKETIME(100, 0, 0) AS time8; # Negative hours are valid; other invalid values return NULL -SELECT MAKETIME(-1, 15, 30) AS invalid_hour; +SELECT CAST(MAKETIME(-1, 15, 30) AS VARCHAR) AS invalid_hour; SELECT MAKETIME(12, 60, 30) AS invalid_minute; SELECT MAKETIME(12, 15, 60) AS invalid_second; SELECT MAKETIME(839, 0, 0) AS invalid_hour_too_large; @@ -29,10 +29,10 @@ SELECT MAKETIME(12, 15, NULL) AS null_second; SELECT MAKETIME(NULL, NULL, NULL) AS null_all; # Float hour and minute values truncate; fractional seconds are preserved -SELECT MAKETIME(12.7, 15.8, 30.9) AS float_values; -SELECT MAKETIME(12, 34, 56.789012) AS fractional_seconds; -SELECT MAKETIME(-12, 34, 56.789012) AS negative_fractional; -SELECT MAKETIME(12, 59, 59.9999996) AS fractional_carry; +SELECT CAST(MAKETIME(12.7, 15.8, 30.9) AS VARCHAR) AS float_values; +SELECT CAST(MAKETIME(12, 34, 56.789012) AS VARCHAR) AS fractional_seconds; +SELECT CAST(MAKETIME(-12, 34, 56.789012) AS VARCHAR) AS negative_fractional; +SELECT CAST(MAKETIME(12, 59, 59.9999996) AS VARCHAR) AS fractional_carry; # Table usage CREATE TABLE t1(h INT, m INT, s INT); @@ -47,6 +47,5 @@ SELECT * FROM t1 WHERE MAKETIME(h, m, s) > '12:00:00'; DROP TABLE t1; # Different numeric types -SELECT MAKETIME(12.0, 15.0, 30.0) AS float_input; +SELECT CAST(MAKETIME(12.0, 15.0, 30.0) AS VARCHAR) AS float_input; SELECT MAKETIME(CAST(12 AS UNSIGNED), CAST(15 AS UNSIGNED), CAST(30 AS UNSIGNED)) AS unsigned_input; - From 996864c27877579bb0e47dbc922024ce1030c434 Mon Sep 17 00:00:00 2001 From: ULookup Date: Fri, 17 Jul 2026 10:54:23 +0800 Subject: [PATCH 13/30] fix: normalize maketime default scale --- pkg/sql/plan/function/function_test.go | 8 ++++++++ pkg/sql/plan/function/list_builtIn.go | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/sql/plan/function/function_test.go b/pkg/sql/plan/function/function_test.go index 4db532491253f..f1ce2a7f0606a 100644 --- a/pkg/sql/plan/function/function_test.go +++ b/pkg/sql/plan/function/function_test.go @@ -373,6 +373,14 @@ func TestMakeTimeReturnScale(t *testing.T) { require.NoError(t, err) require.True(t, fractionalResult.needCast) require.Equal(t, types.T_time.ToTypeWithScale(6), fractionalResult.retType) + + defaultFloatResult, err := GetFunctionByName(proc.Ctx, "maketime", []types.Type{ + types.T_int64.ToType(), + types.T_int64.ToType(), + types.Type{Oid: types.T_float64, Size: 8, Scale: -1}, + }) + require.NoError(t, err) + require.Equal(t, types.T_time.ToTypeWithScale(6), defaultFloatResult.retType) } func TestGetFunctionByNameAESDecryptReturnsBlob(t *testing.T) { diff --git a/pkg/sql/plan/function/list_builtIn.go b/pkg/sql/plan/function/list_builtIn.go index bbeb081537f48..658bb712679f1 100644 --- a/pkg/sql/plan/function/list_builtIn.go +++ b/pkg/sql/plan/function/list_builtIn.go @@ -10740,7 +10740,7 @@ var supportedDateAndTimeBuiltIns = []FuncNew{ func makeTimeReturnType(parameters []types.Type) types.Type { scale := parameters[2].Scale if scale < 0 { - scale = 0 + scale = 6 } else if scale > 6 { scale = 6 } From ec6494cfdd0134a32968f1e03c928eaa7f5adeda Mon Sep 17 00:00:00 2001 From: ULookup Date: Fri, 17 Jul 2026 11:02:38 +0800 Subject: [PATCH 14/30] fix: clamp maketime overflow to endpoint --- pkg/sql/plan/function/func_binary.go | 26 +++++++++++++---------- pkg/sql/plan/function/func_binary_test.go | 22 +++++++++++-------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 19c9b0301e891..2d5760151a17a 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7281,13 +7281,17 @@ func makeTimeIntegerSecond(value int64, null bool) (int64, uint32, bool) { } // makeTimeFromInt64: Helper function to create Time from int64 values -func makeTimeFromInt64(hour, minute, second int64, microsecond uint32, rs *vector.FunctionResult[types.Time], i uint64) error { - if hour < -838 || hour > 838 { +func makeTimeFromInt64(hour, minute, second int64, microsecond uint32, rs *vector.FunctionResult[types.Time]) error { + if minute < 0 || minute > 59 || second < 0 || second > 60 || microsecond >= types.MicroSecsPerSec { return rs.Append(types.Time(0), true) } - if minute < 0 || minute > 59 || second < 0 || second > 60 || microsecond >= types.MicroSecsPerSec { - return rs.Append(types.Time(0), true) + maxTime := types.TimeFromClock(false, 838, 59, 59, 0) + if hour > 838 { + return rs.Append(maxTime, false) + } + if hour < -838 { + return rs.Append(-maxTime, false) } isNegative := hour < 0 @@ -7295,10 +7299,10 @@ func makeTimeFromInt64(hour, minute, second int64, microsecond uint32, rs *vecto hour = -hour } timeValue := types.TimeFromClock(isNegative, uint64(hour), uint8(minute), uint8(second), microsecond) - - normalizedHour, _, _, _, _ := timeValue.ClockFormat() - if normalizedHour > 838 { - return rs.Append(types.Time(0), true) + if timeValue > maxTime { + timeValue = maxTime + } else if timeValue < -maxTime { + timeValue = -maxTime } return rs.Append(timeValue, false) @@ -7348,8 +7352,8 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr hourParam := vector.GenerateFunctionFixedTypeParameter[uint64](ivecs[0]) getHourValue = func(i uint64) (int64, bool) { val, null := hourParam.GetValue(i) - if null || val > 838 { - return 0, true + if val > math.MaxInt64 { + return math.MaxInt64, null } return int64(val), null } @@ -7464,7 +7468,7 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr continue } - if err := makeTimeFromInt64(hourInt, minuteInt, secondInt, microsecond, rs, i); err != nil { + if err := makeTimeFromInt64(hourInt, minuteInt, secondInt, microsecond, rs); err != nil { return err } } diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index 608f606f1d997..72b98e5b81e4c 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9507,25 +9507,29 @@ func TestMakeTimeFractionAndSign(t *testing.T) { fcTC := NewFunctionTestCase(proc, []FunctionTestInput{ NewFunctionTestInput(floatWithMicrosecondScale, - []float64{12, -12, 12, 838, 12, 12}, - []bool{false, false, false, false, false, false}), + []float64{12, -12, 12, 838, -838, 839, -839, 12, 12, 12}, + []bool{false, false, false, false, false, false, false, false, false, false}), NewFunctionTestInput(floatWithMicrosecondScale, - []float64{34, 34, 59, 59, 34, 34}, - []bool{false, false, false, false, false, false}), + []float64{34, 34, 59, 59, 59, 0, 0, 60, 34, 34}, + []bool{false, false, false, false, false, false, false, false, false, false}), NewFunctionTestInput(floatWithMicrosecondScale, - []float64{56.789012, 56.789012, 59.9999996, 59.9999996, math.NaN(), math.Inf(1)}, - []bool{false, false, false, false, false, false}), + []float64{56.789012, 56.789012, 59.9999996, 59.9999996, 59.9999996, 0, 0, 0, math.NaN(), math.Inf(1)}, + []bool{false, false, false, false, false, false, false, false, false, false}), }, NewFunctionTestResult(types.T_time.ToTypeWithScale(6), false, []types.Time{ types.TimeFromClock(false, 12, 34, 56, 789012), types.TimeFromClock(true, 12, 34, 56, 789012), types.TimeFromClock(false, 13, 0, 0, 0), + types.TimeFromClock(false, 838, 59, 59, 0), + types.TimeFromClock(true, 838, 59, 59, 0), + types.TimeFromClock(false, 838, 59, 59, 0), + types.TimeFromClock(true, 838, 59, 59, 0), 0, 0, 0, }, - []bool{false, false, false, true, true, true}), + []bool{false, false, false, false, false, false, false, true, true, true}), MakeTime) s, info := fcTC.Run() @@ -9549,9 +9553,9 @@ func TestMakeTimeUnsignedHourOverflow(t *testing.T) { NewFunctionTestResult(types.T_time.ToType(), false, []types.Time{ types.TimeFromClock(false, 838, 34, 56, 0), - 0, + types.TimeFromClock(false, 838, 59, 59, 0), }, - []bool{false, true}), + []bool{false, false}), MakeTime) s, info := fcTC.Run() From 8d557632fdc789d5fd1c0f897d46db4b8a37986d Mon Sep 17 00:00:00 2001 From: ULookup Date: Fri, 17 Jul 2026 11:14:00 +0800 Subject: [PATCH 15/30] fix: normalize maketime float hour overflow --- pkg/sql/plan/function/func_binary.go | 12 +++++- pkg/sql/plan/function/func_binary_test.go | 48 +++++++++++++++++++---- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 2d5760151a17a..1f186788cf635 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7361,7 +7361,17 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr hourParam := vector.GenerateFunctionFixedTypeParameter[float64](ivecs[0]) getHourValue = func(i uint64) (int64, bool) { val, null := hourParam.GetValue(i) - return int64(val), null // Truncate decimal part + if null || math.IsNaN(val) || math.IsInf(val, 0) { + return 0, true + } + val = math.Trunc(val) + if val >= float64(math.MaxInt64) { + return math.MaxInt64, false + } + if val <= float64(math.MinInt64) { + return math.MinInt64, false + } + return int64(val), false } default: return moerr.NewInvalidArgNoCtx("MAKETIME hour parameter", hourType) diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index 72b98e5b81e4c..6cdd84c2e0854 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9507,14 +9507,14 @@ func TestMakeTimeFractionAndSign(t *testing.T) { fcTC := NewFunctionTestCase(proc, []FunctionTestInput{ NewFunctionTestInput(floatWithMicrosecondScale, - []float64{12, -12, 12, 838, -838, 839, -839, 12, 12, 12}, - []bool{false, false, false, false, false, false, false, false, false, false}), + []float64{12, -12, 12, 838, -838, 839, -839, 12, 12, 12, math.MaxFloat64, -math.MaxFloat64, 838.9, -838.9, math.NaN(), math.Inf(1), math.Inf(-1), 0}, + []bool{false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, true}), NewFunctionTestInput(floatWithMicrosecondScale, - []float64{34, 34, 59, 59, 59, 0, 0, 60, 34, 34}, - []bool{false, false, false, false, false, false, false, false, false, false}), + []float64{34, 34, 59, 59, 59, 0, 0, 60, 34, 34, 0, 0, 0, 0, 0, 0, 0, 0}, + []bool{false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false}), NewFunctionTestInput(floatWithMicrosecondScale, - []float64{56.789012, 56.789012, 59.9999996, 59.9999996, 59.9999996, 0, 0, 0, math.NaN(), math.Inf(1)}, - []bool{false, false, false, false, false, false, false, false, false, false}), + []float64{56.789012, 56.789012, 59.9999996, 59.9999996, 59.9999996, 0, 0, 0, math.NaN(), math.Inf(1), 0, 0, 0, 0, 0, 0, 0, 0}, + []bool{false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false}), }, NewFunctionTestResult(types.T_time.ToTypeWithScale(6), false, []types.Time{ @@ -9528,8 +9528,16 @@ func TestMakeTimeFractionAndSign(t *testing.T) { 0, 0, 0, + types.TimeFromClock(false, 838, 59, 59, 0), + types.TimeFromClock(true, 838, 59, 59, 0), + types.TimeFromClock(false, 838, 0, 0, 0), + types.TimeFromClock(true, 838, 0, 0, 0), + 0, + 0, + 0, + 0, }, - []bool{false, false, false, false, false, false, false, true, true, true}), + []bool{false, false, false, false, false, false, false, true, true, true, false, false, false, false, true, true, true, true}), MakeTime) s, info := fcTC.Run() @@ -9562,6 +9570,32 @@ func TestMakeTimeUnsignedHourOverflow(t *testing.T) { require.True(t, s, "MAKETIME unsigned hour overflow case failed: %s", info) } +func TestMakeTimeSignedHourOverflow(t *testing.T) { + proc := testutil.NewProcess(t) + fcTC := NewFunctionTestCase(proc, + []FunctionTestInput{ + NewFunctionTestInput(types.T_int64.ToType(), + []int64{math.MaxInt64, math.MinInt64}, + []bool{false, false}), + NewFunctionTestInput(types.T_int64.ToType(), + []int64{0, 0}, + []bool{false, false}), + NewFunctionTestInput(types.T_int64.ToType(), + []int64{0, 0}, + []bool{false, false}), + }, + NewFunctionTestResult(types.T_time.ToType(), false, + []types.Time{ + types.TimeFromClock(false, 838, 59, 59, 0), + types.TimeFromClock(true, 838, 59, 59, 0), + }, + []bool{false, false}), + MakeTime) + + s, info := fcTC.Run() + require.True(t, s, "MAKETIME signed hour overflow case failed: %s", info) +} + func TestMakeTimeIntegerSecondRange(t *testing.T) { proc := testutil.NewProcess(t) expected := NewFunctionTestResult(types.T_time.ToType(), false, From 1046691b8f5e9c0af8b3e300a59a9a06c5ceeedc Mon Sep 17 00:00:00 2001 From: ULookup Date: Fri, 17 Jul 2026 11:27:35 +0800 Subject: [PATCH 16/30] test: cover maketime SQL boundaries --- .../function/func_datetime_maketime.result | 21 ++++++++++++++++--- .../function/func_datetime_maketime.test | 13 ++++++++++-- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index 8d07f31630d13..5bca33d432a35 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -25,6 +25,12 @@ time7 SELECT MAKETIME(100, 0, 0) AS time8; time8 100:00:00 +SELECT MAKETIME(839, 0, 0) AS oversized_positive_hour; +oversized_positive_hour +838:59:59 +SELECT MAKETIME(-839, 0, 0) AS oversized_negative_hour; +oversized_negative_hour +-838:59:59 SELECT CAST(MAKETIME(-1, 15, 30) AS VARCHAR) AS invalid_hour; invalid_hour -01:15:30 @@ -34,9 +40,6 @@ null SELECT MAKETIME(12, 15, 60) AS invalid_second; invalid_second null -SELECT MAKETIME(839, 0, 0) AS invalid_hour_too_large; -invalid_hour_too_large -null SELECT MAKETIME(NULL, 15, 30) AS null_hour; null_hour null @@ -61,6 +64,18 @@ negative_fractional SELECT CAST(MAKETIME(12, 59, 59.9999996) AS VARCHAR) AS fractional_carry; fractional_carry 13:00:00.000000 +SELECT CAST(MAKETIME(838, 59, 59.9999996) AS VARCHAR) AS positive_rounded_endpoint; +positive_rounded_endpoint +838:59:59.000000 +SELECT CAST(MAKETIME(-838, 59, 59.9999996) AS VARCHAR) AS negative_rounded_endpoint; +negative_rounded_endpoint +-838:59:59.000000 +CREATE TABLE maketime_double_seconds(h INT, m INT, s DOUBLE); +INSERT INTO maketime_double_seconds VALUES (12, 34, 56.789012); +SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS double_column_microseconds FROM maketime_double_seconds; +double_column_microseconds +12:34:56.789012 +DROP TABLE maketime_double_seconds; CREATE TABLE t1(h INT, m INT, s INT); INSERT INTO t1 VALUES (12, 15, 30), (0, 0, 0), (23, 59, 59); SELECT MAKETIME(h, m, s) AS time_value FROM t1; diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index 1f4dd7326ed4c..e6fb38e64c291 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -12,15 +12,16 @@ SELECT MAKETIME(0, 30, 45) AS time4; SELECT MAKETIME(12, 0, 0) AS time5; SELECT MAKETIME(12, 15, 0) AS time6; -# Large hour values (MySQL TIME allows up to 838 hours) +# Large hour values clamp to the MySQL TIME endpoint SELECT MAKETIME(838, 59, 59) AS time7; SELECT MAKETIME(100, 0, 0) AS time8; +SELECT MAKETIME(839, 0, 0) AS oversized_positive_hour; +SELECT MAKETIME(-839, 0, 0) AS oversized_negative_hour; # Negative hours are valid; other invalid values return NULL SELECT CAST(MAKETIME(-1, 15, 30) AS VARCHAR) AS invalid_hour; SELECT MAKETIME(12, 60, 30) AS invalid_minute; SELECT MAKETIME(12, 15, 60) AS invalid_second; -SELECT MAKETIME(839, 0, 0) AS invalid_hour_too_large; # NULL handling SELECT MAKETIME(NULL, 15, 30) AS null_hour; @@ -33,6 +34,14 @@ SELECT CAST(MAKETIME(12.7, 15.8, 30.9) AS VARCHAR) AS float_values; SELECT CAST(MAKETIME(12, 34, 56.789012) AS VARCHAR) AS fractional_seconds; SELECT CAST(MAKETIME(-12, 34, 56.789012) AS VARCHAR) AS negative_fractional; SELECT CAST(MAKETIME(12, 59, 59.9999996) AS VARCHAR) AS fractional_carry; +SELECT CAST(MAKETIME(838, 59, 59.9999996) AS VARCHAR) AS positive_rounded_endpoint; +SELECT CAST(MAKETIME(-838, 59, 59.9999996) AS VARCHAR) AS negative_rounded_endpoint; + +# Default DOUBLE column retains microseconds +CREATE TABLE maketime_double_seconds(h INT, m INT, s DOUBLE); +INSERT INTO maketime_double_seconds VALUES (12, 34, 56.789012); +SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS double_column_microseconds FROM maketime_double_seconds; +DROP TABLE maketime_double_seconds; # Table usage CREATE TABLE t1(h INT, m INT, s INT); From cf27d9563987e160ae4bdd7517df7f6036fa942e Mon Sep 17 00:00:00 2001 From: ULookup Date: Fri, 17 Jul 2026 11:41:34 +0800 Subject: [PATCH 17/30] fix: validate maketime float minute input --- pkg/sql/plan/function/func_binary.go | 9 +++++- pkg/sql/plan/function/func_binary_test.go | 32 +++++++++++++++++++ .../function/func_datetime_maketime.result | 4 +-- .../function/func_datetime_maketime.test | 2 +- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 1f186788cf635..38ba5aef12efc 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7413,7 +7413,14 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr minuteParam := vector.GenerateFunctionFixedTypeParameter[float64](ivecs[1]) getMinuteValue = func(i uint64) (int64, bool) { val, null := minuteParam.GetValue(i) - return int64(val), null // Truncate decimal part + if null || math.IsNaN(val) || math.IsInf(val, 0) { + return 0, true + } + val = math.Trunc(val) + if val < 0 || val >= 60 { + return 0, true + } + return int64(val), false } default: return moerr.NewInvalidArgNoCtx("MAKETIME minute parameter", minuteType) diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index 6cdd84c2e0854..1b4ae8cb15fd9 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9596,6 +9596,38 @@ func TestMakeTimeSignedHourOverflow(t *testing.T) { require.True(t, s, "MAKETIME signed hour overflow case failed: %s", info) } +func TestMakeTimeFloatMinuteRange(t *testing.T) { + proc := testutil.NewProcess(t) + fcTC := NewFunctionTestCase(proc, + []FunctionTestInput{ + NewFunctionTestInput(types.T_int64.ToType(), + []int64{12, 12, 12, 12, 12, 12, 12, 12}, + []bool{false, false, false, false, false, false, false, false}), + NewFunctionTestInput(types.T_float64.ToType(), + []float64{math.NaN(), math.Inf(1), math.Inf(-1), math.MaxFloat64, -math.MaxFloat64, 15.8, 59.9, -0.9}, + []bool{false, false, false, false, false, false, false, false}), + NewFunctionTestInput(types.T_int64.ToType(), + []int64{0, 0, 0, 0, 0, 0, 0, 0}, + []bool{false, false, false, false, false, false, false, false}), + }, + NewFunctionTestResult(types.T_time.ToType(), false, + []types.Time{ + 0, + 0, + 0, + 0, + 0, + types.TimeFromClock(false, 12, 15, 0, 0), + types.TimeFromClock(false, 12, 59, 0, 0), + types.TimeFromClock(false, 12, 0, 0, 0), + }, + []bool{true, true, true, true, true, false, false, false}), + MakeTime) + + s, info := fcTC.Run() + require.True(t, s, "MAKETIME float minute range failed: %s", info) +} + func TestMakeTimeIntegerSecondRange(t *testing.T) { proc := testutil.NewProcess(t) expected := NewFunctionTestResult(types.T_time.ToType(), false, diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index 5bca33d432a35..cfb5584a65704 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -31,8 +31,8 @@ oversized_positive_hour SELECT MAKETIME(-839, 0, 0) AS oversized_negative_hour; oversized_negative_hour -838:59:59 -SELECT CAST(MAKETIME(-1, 15, 30) AS VARCHAR) AS invalid_hour; -invalid_hour +SELECT CAST(MAKETIME(-1, 15, 30) AS VARCHAR) AS negative_hour; +negative_hour -01:15:30 SELECT MAKETIME(12, 60, 30) AS invalid_minute; invalid_minute diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index e6fb38e64c291..1efd1ee8cf4ae 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -19,7 +19,7 @@ SELECT MAKETIME(839, 0, 0) AS oversized_positive_hour; SELECT MAKETIME(-839, 0, 0) AS oversized_negative_hour; # Negative hours are valid; other invalid values return NULL -SELECT CAST(MAKETIME(-1, 15, 30) AS VARCHAR) AS invalid_hour; +SELECT CAST(MAKETIME(-1, 15, 30) AS VARCHAR) AS negative_hour; SELECT MAKETIME(12, 60, 30) AS invalid_minute; SELECT MAKETIME(12, 15, 60) AS invalid_second; From 23da2747d3d17d8e832ce68b2881f1fae9ce9010 Mon Sep 17 00:00:00 2001 From: ULookup Date: Fri, 17 Jul 2026 13:10:58 +0800 Subject: [PATCH 18/30] test: format maketime scale regression --- pkg/sql/plan/function/function_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sql/plan/function/function_test.go b/pkg/sql/plan/function/function_test.go index f1ce2a7f0606a..318331185327e 100644 --- a/pkg/sql/plan/function/function_test.go +++ b/pkg/sql/plan/function/function_test.go @@ -377,7 +377,7 @@ func TestMakeTimeReturnScale(t *testing.T) { defaultFloatResult, err := GetFunctionByName(proc.Ctx, "maketime", []types.Type{ types.T_int64.ToType(), types.T_int64.ToType(), - types.Type{Oid: types.T_float64, Size: 8, Scale: -1}, + {Oid: types.T_float64, Size: 8, Scale: -1}, }) require.NoError(t, err) require.Equal(t, types.T_time.ToTypeWithScale(6), defaultFloatResult.retType) From e03eaf88114a748a426af882f77a9cdd47ab3a6f Mon Sep 17 00:00:00 2001 From: ULookup Date: Fri, 17 Jul 2026 14:55:21 +0800 Subject: [PATCH 19/30] fix: align maketime numeric conversion --- pkg/sql/plan/function/func_binary.go | 225 ++++++++---------- pkg/sql/plan/function/func_binary_test.go | 72 +++++- .../function/func_datetime_maketime.result | 2 +- 3 files changed, 159 insertions(+), 140 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 38ba5aef12efc..fb8b8c1296f8c 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7308,171 +7308,142 @@ func makeTimeFromInt64(hour, minute, second int64, microsecond uint32, rs *vecto return rs.Append(timeValue, false) } +func makeTimeSignedIntegerGetter[T constraints.Signed](vec *vector.Vector) func(uint64) (int64, bool) { + param := vector.GenerateFunctionFixedTypeParameter[T](vec) + return func(i uint64) (int64, bool) { + value, null := param.GetValue(i) + return int64(value), null + } +} + +func makeTimeUnsignedIntegerGetter[T constraints.Unsigned](vec *vector.Vector) func(uint64) (int64, bool) { + param := vector.GenerateFunctionFixedTypeParameter[T](vec) + return func(i uint64) (int64, bool) { + value, null := param.GetValue(i) + if uint64(value) > math.MaxInt64 { + return math.MaxInt64, null + } + return int64(value), null + } +} + +func makeTimeIntegerGetter(vec *vector.Vector) (func(uint64) (int64, bool), bool) { + switch vec.GetType().Oid { + case types.T_int8: + return makeTimeSignedIntegerGetter[int8](vec), true + case types.T_int16: + return makeTimeSignedIntegerGetter[int16](vec), true + case types.T_int32: + return makeTimeSignedIntegerGetter[int32](vec), true + case types.T_int64: + return makeTimeSignedIntegerGetter[int64](vec), true + case types.T_uint8: + return makeTimeUnsignedIntegerGetter[uint8](vec), true + case types.T_uint16: + return makeTimeUnsignedIntegerGetter[uint16](vec), true + case types.T_uint32: + return makeTimeUnsignedIntegerGetter[uint32](vec), true + case types.T_uint64: + return makeTimeUnsignedIntegerGetter[uint64](vec), true + default: + return nil, false + } +} + +func makeTimeFloatGetter[T constraints.Float](vec *vector.Vector) func(uint64) (float64, bool) { + param := vector.GenerateFunctionFixedTypeParameter[T](vec) + return func(i uint64) (float64, bool) { + value, null := param.GetValue(i) + return float64(value), null + } +} + // MakeTime: MAKETIME(hour, minute, second) - Returns a time value calculated from the hour, minute, and second arguments. func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *process.Process, length int, selectList *FunctionSelectList) error { rs := vector.MustFunctionResult[types.Time](result) - // Check the types of input vectors and create appropriate parameter wrappers - hourType := ivecs[0].GetType().Oid - minuteType := ivecs[1].GetType().Oid - secondType := ivecs[2].GetType().Oid - - // Create parameter wrappers based on types (these can be reused for all rows) var getHourValue func(uint64) (int64, bool) var getMinuteValue func(uint64) (int64, bool) var getSecondValue func(uint64) (int64, uint32, bool) - // Setup hour parameter extractor - switch hourType { - case types.T_int8: - hourParam := vector.GenerateFunctionFixedTypeParameter[int8](ivecs[0]) - getHourValue = func(i uint64) (int64, bool) { - val, null := hourParam.GetValue(i) - return int64(val), null - } - case types.T_int16: - hourParam := vector.GenerateFunctionFixedTypeParameter[int16](ivecs[0]) - getHourValue = func(i uint64) (int64, bool) { - val, null := hourParam.GetValue(i) - return int64(val), null - } - case types.T_int32: - hourParam := vector.GenerateFunctionFixedTypeParameter[int32](ivecs[0]) - getHourValue = func(i uint64) (int64, bool) { - val, null := hourParam.GetValue(i) - return int64(val), null - } - case types.T_int64: - hourParam := vector.GenerateFunctionFixedTypeParameter[int64](ivecs[0]) - getHourValue = func(i uint64) (int64, bool) { - val, null := hourParam.GetValue(i) - return val, null - } - case types.T_uint8, types.T_uint16, types.T_uint32, types.T_uint64: - hourParam := vector.GenerateFunctionFixedTypeParameter[uint64](ivecs[0]) - getHourValue = func(i uint64) (int64, bool) { - val, null := hourParam.GetValue(i) - if val > math.MaxInt64 { - return math.MaxInt64, null - } - return int64(val), null + if getter, ok := makeTimeIntegerGetter(ivecs[0]); ok { + getHourValue = getter + } else { + var getFloat func(uint64) (float64, bool) + switch ivecs[0].GetType().Oid { + case types.T_float32: + getFloat = makeTimeFloatGetter[float32](ivecs[0]) + case types.T_float64: + getFloat = makeTimeFloatGetter[float64](ivecs[0]) + default: + return moerr.NewInvalidArgNoCtx("MAKETIME hour parameter", ivecs[0].GetType().Oid) } - case types.T_float32, types.T_float64: - hourParam := vector.GenerateFunctionFixedTypeParameter[float64](ivecs[0]) getHourValue = func(i uint64) (int64, bool) { - val, null := hourParam.GetValue(i) - if null || math.IsNaN(val) || math.IsInf(val, 0) { + value, null := getFloat(i) + if null || math.IsNaN(value) || math.IsInf(value, 0) { return 0, true } - val = math.Trunc(val) - if val >= float64(math.MaxInt64) { + rounded := math.RoundToEven(value) + if rounded >= float64(math.MaxInt64) { return math.MaxInt64, false } - if val <= float64(math.MinInt64) { + if rounded <= float64(math.MinInt64) { return math.MinInt64, false } - return int64(val), false + return int64(rounded), false } - default: - return moerr.NewInvalidArgNoCtx("MAKETIME hour parameter", hourType) } - // Setup minute parameter extractor - switch minuteType { - case types.T_int8: - minuteParam := vector.GenerateFunctionFixedTypeParameter[int8](ivecs[1]) - getMinuteValue = func(i uint64) (int64, bool) { - val, null := minuteParam.GetValue(i) - return int64(val), null - } - case types.T_int16: - minuteParam := vector.GenerateFunctionFixedTypeParameter[int16](ivecs[1]) - getMinuteValue = func(i uint64) (int64, bool) { - val, null := minuteParam.GetValue(i) - return int64(val), null - } - case types.T_int32: - minuteParam := vector.GenerateFunctionFixedTypeParameter[int32](ivecs[1]) - getMinuteValue = func(i uint64) (int64, bool) { - val, null := minuteParam.GetValue(i) - return int64(val), null - } - case types.T_int64: - minuteParam := vector.GenerateFunctionFixedTypeParameter[int64](ivecs[1]) - getMinuteValue = func(i uint64) (int64, bool) { - val, null := minuteParam.GetValue(i) - return val, null - } - case types.T_uint8, types.T_uint16, types.T_uint32, types.T_uint64: - minuteParam := vector.GenerateFunctionFixedTypeParameter[uint64](ivecs[1]) - getMinuteValue = func(i uint64) (int64, bool) { - val, null := minuteParam.GetValue(i) - return int64(val), null + if getter, ok := makeTimeIntegerGetter(ivecs[1]); ok { + getMinuteValue = getter + } else { + var getFloat func(uint64) (float64, bool) + switch ivecs[1].GetType().Oid { + case types.T_float32: + getFloat = makeTimeFloatGetter[float32](ivecs[1]) + case types.T_float64: + getFloat = makeTimeFloatGetter[float64](ivecs[1]) + default: + return moerr.NewInvalidArgNoCtx("MAKETIME minute parameter", ivecs[1].GetType().Oid) } - case types.T_float32, types.T_float64: - minuteParam := vector.GenerateFunctionFixedTypeParameter[float64](ivecs[1]) getMinuteValue = func(i uint64) (int64, bool) { - val, null := minuteParam.GetValue(i) - if null || math.IsNaN(val) || math.IsInf(val, 0) { + value, null := getFloat(i) + if null || math.IsNaN(value) || math.IsInf(value, 0) { return 0, true } - val = math.Trunc(val) - if val < 0 || val >= 60 { + rounded := math.RoundToEven(value) + if rounded < 0 || rounded >= 60 { return 0, true } - return int64(val), false + return int64(rounded), false } - default: - return moerr.NewInvalidArgNoCtx("MAKETIME minute parameter", minuteType) } - // Setup second parameter extractor - switch secondType { - case types.T_int8: - secondParam := vector.GenerateFunctionFixedTypeParameter[int8](ivecs[2]) + if getter, ok := makeTimeIntegerGetter(ivecs[2]); ok { getSecondValue = func(i uint64) (int64, uint32, bool) { - val, null := secondParam.GetValue(i) - return makeTimeIntegerSecond(int64(val), null) + value, null := getter(i) + return makeTimeIntegerSecond(value, null) } - case types.T_int16: - secondParam := vector.GenerateFunctionFixedTypeParameter[int16](ivecs[2]) - getSecondValue = func(i uint64) (int64, uint32, bool) { - val, null := secondParam.GetValue(i) - return makeTimeIntegerSecond(int64(val), null) - } - case types.T_int32: - secondParam := vector.GenerateFunctionFixedTypeParameter[int32](ivecs[2]) - getSecondValue = func(i uint64) (int64, uint32, bool) { - val, null := secondParam.GetValue(i) - return makeTimeIntegerSecond(int64(val), null) - } - case types.T_int64: - secondParam := vector.GenerateFunctionFixedTypeParameter[int64](ivecs[2]) - getSecondValue = func(i uint64) (int64, uint32, bool) { - val, null := secondParam.GetValue(i) - return makeTimeIntegerSecond(val, null) - } - case types.T_uint8, types.T_uint16, types.T_uint32, types.T_uint64: - secondParam := vector.GenerateFunctionFixedTypeParameter[uint64](ivecs[2]) - getSecondValue = func(i uint64) (int64, uint32, bool) { - val, null := secondParam.GetValue(i) - return makeTimeIntegerSecond(int64(val), null) + } else { + var getFloat func(uint64) (float64, bool) + switch ivecs[2].GetType().Oid { + case types.T_float32: + getFloat = makeTimeFloatGetter[float32](ivecs[2]) + case types.T_float64: + getFloat = makeTimeFloatGetter[float64](ivecs[2]) + default: + return moerr.NewInvalidArgNoCtx("MAKETIME second parameter", ivecs[2].GetType().Oid) } - case types.T_float32, types.T_float64: - secondParam := vector.GenerateFunctionFixedTypeParameter[float64](ivecs[2]) getSecondValue = func(i uint64) (int64, uint32, bool) { - val, null := secondParam.GetValue(i) - if null || math.IsNaN(val) || math.IsInf(val, 0) || val < 0 || val >= 60 { + value, null := getFloat(i) + if null || math.IsNaN(value) || math.IsInf(value, 0) || value < 0 || value >= 60 { return 0, 0, true } - total := int64(math.Round(val * float64(types.MicroSecsPerSec))) + total := int64(math.Round(value * float64(types.MicroSecsPerSec))) return total / types.MicroSecsPerSec, uint32(total % types.MicroSecsPerSec), false } - default: - return moerr.NewInvalidArgNoCtx("MAKETIME second parameter", secondType) } - // Process all rows for i := uint64(0); i < uint64(length); i++ { hourInt, null1 := getHourValue(i) minuteInt, null2 := getMinuteValue(i) diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index 1b4ae8cb15fd9..0a43a72cbde5f 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9530,8 +9530,8 @@ func TestMakeTimeFractionAndSign(t *testing.T) { 0, types.TimeFromClock(false, 838, 59, 59, 0), types.TimeFromClock(true, 838, 59, 59, 0), - types.TimeFromClock(false, 838, 0, 0, 0), - types.TimeFromClock(true, 838, 0, 0, 0), + types.TimeFromClock(false, 838, 59, 59, 0), + types.TimeFromClock(true, 838, 59, 59, 0), 0, 0, 0, @@ -9596,32 +9596,80 @@ func TestMakeTimeSignedHourOverflow(t *testing.T) { require.True(t, s, "MAKETIME signed hour overflow case failed: %s", info) } +func TestMakeTimeUint32Overload(t *testing.T) { + proc := testutil.NewProcess(t) + fcTC := NewFunctionTestCase(proc, + []FunctionTestInput{ + NewFunctionTestInput(types.T_uint32.ToType(), []uint32{12}, []bool{false}), + NewFunctionTestInput(types.T_uint32.ToType(), []uint32{34}, []bool{false}), + NewFunctionTestInput(types.T_uint32.ToType(), []uint32{56}, []bool{false}), + }, + NewFunctionTestResult(types.T_time.ToType(), false, + []types.Time{types.TimeFromClock(false, 12, 34, 56, 0)}, + []bool{false}), + MakeTime) + + s, info := fcTC.Run() + require.True(t, s, "MAKETIME uint32 overload failed: %s", info) +} + +func TestMakeTimeFloatHourRounding(t *testing.T) { + proc := testutil.NewProcess(t) + fcTC := NewFunctionTestCase(proc, + []FunctionTestInput{ + NewFunctionTestInput(types.T_float64.ToType(), + []float64{12.7, 12.5, 13.5, -12.5, -13.5, 838.9}, + []bool{false, false, false, false, false, false}), + NewFunctionTestInput(types.T_int64.ToType(), + []int64{0, 0, 0, 0, 0, 0}, + []bool{false, false, false, false, false, false}), + NewFunctionTestInput(types.T_int64.ToType(), + []int64{0, 0, 0, 0, 0, 0}, + []bool{false, false, false, false, false, false}), + }, + NewFunctionTestResult(types.T_time.ToType(), false, + []types.Time{ + types.TimeFromClock(false, 13, 0, 0, 0), + types.TimeFromClock(false, 12, 0, 0, 0), + types.TimeFromClock(false, 14, 0, 0, 0), + types.TimeFromClock(true, 12, 0, 0, 0), + types.TimeFromClock(true, 14, 0, 0, 0), + types.TimeFromClock(false, 838, 59, 59, 0), + }, + []bool{false, false, false, false, false, false}), + MakeTime) + + s, info := fcTC.Run() + require.True(t, s, "MAKETIME float hour rounding failed: %s", info) +} + func TestMakeTimeFloatMinuteRange(t *testing.T) { proc := testutil.NewProcess(t) fcTC := NewFunctionTestCase(proc, []FunctionTestInput{ NewFunctionTestInput(types.T_int64.ToType(), - []int64{12, 12, 12, 12, 12, 12, 12, 12}, - []bool{false, false, false, false, false, false, false, false}), + []int64{12, 12, 12, 12, 12, 12, 12, 12, 12}, + []bool{false, false, false, false, false, false, false, false, false}), NewFunctionTestInput(types.T_float64.ToType(), - []float64{math.NaN(), math.Inf(1), math.Inf(-1), math.MaxFloat64, -math.MaxFloat64, 15.8, 59.9, -0.9}, - []bool{false, false, false, false, false, false, false, false}), + []float64{15.8, 58.5, 59.5, 59.9, -0.5, -0.9, math.NaN(), math.Inf(1), math.Inf(-1)}, + []bool{false, false, false, false, false, false, false, false, false}), NewFunctionTestInput(types.T_int64.ToType(), - []int64{0, 0, 0, 0, 0, 0, 0, 0}, - []bool{false, false, false, false, false, false, false, false}), + []int64{0, 0, 0, 0, 0, 0, 0, 0, 0}, + []bool{false, false, false, false, false, false, false, false, false}), }, NewFunctionTestResult(types.T_time.ToType(), false, []types.Time{ + types.TimeFromClock(false, 12, 16, 0, 0), + types.TimeFromClock(false, 12, 58, 0, 0), + 0, 0, + types.TimeFromClock(false, 12, 0, 0, 0), 0, 0, 0, 0, - types.TimeFromClock(false, 12, 15, 0, 0), - types.TimeFromClock(false, 12, 59, 0, 0), - types.TimeFromClock(false, 12, 0, 0, 0), }, - []bool{true, true, true, true, true, false, false, false}), + []bool{false, false, true, true, false, true, true, true, true}), MakeTime) s, info := fcTC.Run() diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index cfb5584a65704..7570ae275d300 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -54,7 +54,7 @@ null_all null SELECT CAST(MAKETIME(12.7, 15.8, 30.9) AS VARCHAR) AS float_values; float_values -12:15:30.9 +13:16:30.9 SELECT CAST(MAKETIME(12, 34, 56.789012) AS VARCHAR) AS fractional_seconds; fractional_seconds 12:34:56.789012 From 62074fc285f9f105eae08bebcb54cc14b7f4cdf6 Mon Sep 17 00:00:00 2001 From: ULookup Date: Fri, 17 Jul 2026 15:29:07 +0800 Subject: [PATCH 20/30] fix: preserve maketime string seconds --- pkg/sql/plan/function/function_test.go | 18 ++++++++++++++++ pkg/sql/plan/function/list_builtIn.go | 21 ++++++++++++++++++- .../function/func_datetime_maketime.result | 9 ++++++++ .../function/func_datetime_maketime.test | 9 +++++++- 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/pkg/sql/plan/function/function_test.go b/pkg/sql/plan/function/function_test.go index 318331185327e..9225842a63b34 100644 --- a/pkg/sql/plan/function/function_test.go +++ b/pkg/sql/plan/function/function_test.go @@ -383,6 +383,24 @@ func TestMakeTimeReturnScale(t *testing.T) { require.Equal(t, types.T_time.ToTypeWithScale(6), defaultFloatResult.retType) } +func TestMakeTimeStringSecondUsesFloatOverload(t *testing.T) { + proc := testutil.NewProcess(t) + + result, err := GetFunctionByName(proc.Ctx, "maketime", []types.Type{ + types.T_int64.ToType(), + types.T_int64.ToType(), + types.T_varchar.ToType(), + }) + require.NoError(t, err) + require.True(t, result.needCast) + require.Len(t, result.targetTypes, 3) + for _, target := range result.targetTypes { + require.Equal(t, types.T_float64, target.Oid) + } + require.Equal(t, int32(-1), result.targetTypes[2].Scale) + require.Equal(t, types.T_time.ToTypeWithScale(6), result.retType) +} + func TestGetFunctionByNameAESDecryptReturnsBlob(t *testing.T) { proc := testutil.NewProcess(t) tests := []struct { diff --git a/pkg/sql/plan/function/list_builtIn.go b/pkg/sql/plan/function/list_builtIn.go index 658bb712679f1..930ca05ff0f71 100644 --- a/pkg/sql/plan/function/list_builtIn.go +++ b/pkg/sql/plan/function/list_builtIn.go @@ -10747,6 +10747,25 @@ func makeTimeReturnType(parameters []types.Type) types.Type { return types.T_time.ToTypeWithScale(scale) } +func makeTimeCheck(overloads []overload, inputs []types.Type) checkResult { + if len(inputs) == 3 && inputs[2].Oid.IsMySQLString() { + for i, ov := range overloads { + if len(ov.args) != 3 || ov.args[0] != types.T_float64 || ov.args[1] != types.T_float64 || ov.args[2] != types.T_float64 { + continue + } + if status, _ := tryToMatch(inputs, ov.args); status != matchFailed { + targets := make([]types.Type, len(inputs)) + for j := range targets { + targets[j] = ov.args[j].ToType() + SetTargetScaleFromSource(&inputs[j], &targets[j]) + } + return newCheckResultWithCast(i, targets) + } + } + } + return fixedTypeMatch(overloads, inputs) +} + var supportedControlBuiltIns = []FuncNew{ // function `add_fault_point` { @@ -11197,7 +11216,7 @@ var supportedControlBuiltIns = []FuncNew{ functionId: MAKETIME, class: plan.Function_STRICT, layout: STANDARD_FUNCTION, - checkFn: fixedTypeMatch, + checkFn: makeTimeCheck, Overloads: []overload{ { overloadId: 0, diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index 7570ae275d300..9518356754a51 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -58,6 +58,9 @@ float_values SELECT CAST(MAKETIME(12, 34, 56.789012) AS VARCHAR) AS fractional_seconds; fractional_seconds 12:34:56.789012 +SELECT CAST(MAKETIME(12, 34, '56.789012') AS VARCHAR) AS string_fractional_seconds; +string_fractional_seconds +12:34:56.789012 SELECT CAST(MAKETIME(-12, 34, 56.789012) AS VARCHAR) AS negative_fractional; negative_fractional -12:34:56.789012 @@ -76,6 +79,12 @@ SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS double_column_microseconds FROM mak double_column_microseconds 12:34:56.789012 DROP TABLE maketime_double_seconds; +CREATE TABLE maketime_varchar_seconds(h INT, m INT, s VARCHAR(20)); +INSERT INTO maketime_varchar_seconds VALUES (12, 34, '56.789012'); +SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS varchar_column_microseconds FROM maketime_varchar_seconds; +varchar_column_microseconds +12:34:56.789012 +DROP TABLE maketime_varchar_seconds; CREATE TABLE t1(h INT, m INT, s INT); INSERT INTO t1 VALUES (12, 15, 30), (0, 0, 0), (23, 59, 59); SELECT MAKETIME(h, m, s) AS time_value FROM t1; diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index 1efd1ee8cf4ae..01e600beb5789 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -29,9 +29,10 @@ SELECT MAKETIME(12, NULL, 30) AS null_minute; SELECT MAKETIME(12, 15, NULL) AS null_second; SELECT MAKETIME(NULL, NULL, NULL) AS null_all; -# Float hour and minute values truncate; fractional seconds are preserved +# Float hour and minute values round to nearest-even; fractional seconds are preserved SELECT CAST(MAKETIME(12.7, 15.8, 30.9) AS VARCHAR) AS float_values; SELECT CAST(MAKETIME(12, 34, 56.789012) AS VARCHAR) AS fractional_seconds; +SELECT CAST(MAKETIME(12, 34, '56.789012') AS VARCHAR) AS string_fractional_seconds; SELECT CAST(MAKETIME(-12, 34, 56.789012) AS VARCHAR) AS negative_fractional; SELECT CAST(MAKETIME(12, 59, 59.9999996) AS VARCHAR) AS fractional_carry; SELECT CAST(MAKETIME(838, 59, 59.9999996) AS VARCHAR) AS positive_rounded_endpoint; @@ -43,6 +44,12 @@ INSERT INTO maketime_double_seconds VALUES (12, 34, 56.789012); SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS double_column_microseconds FROM maketime_double_seconds; DROP TABLE maketime_double_seconds; +# VARCHAR second values retain fractional microseconds +CREATE TABLE maketime_varchar_seconds(h INT, m INT, s VARCHAR(20)); +INSERT INTO maketime_varchar_seconds VALUES (12, 34, '56.789012'); +SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS varchar_column_microseconds FROM maketime_varchar_seconds; +DROP TABLE maketime_varchar_seconds; + # Table usage CREATE TABLE t1(h INT, m INT, s INT); INSERT INTO t1 VALUES (12, 15, 30), (0, 0, 0), (23, 59, 59); From 8f00784294b6fe4a41bef1f40b6b73dbf6ebd012 Mon Sep 17 00:00:00 2001 From: ULookup Date: Fri, 17 Jul 2026 15:58:23 +0800 Subject: [PATCH 21/30] fix: preserve maketime string argument semantics --- pkg/sql/plan/function/func_binary.go | 31 ++++++++- pkg/sql/plan/function/func_binary_test.go | 49 ++++++++++++++ pkg/sql/plan/function/function_test.go | 67 +++++++++++++++++++ pkg/sql/plan/function/list_builtIn.go | 67 +++++++++++++++---- .../function/func_datetime_maketime.result | 15 +++++ .../function/func_datetime_maketime.test | 9 +++ 6 files changed, 222 insertions(+), 16 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index fb8b8c1296f8c..c424ebb2a5e4c 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7358,6 +7358,29 @@ func makeTimeFloatGetter[T constraints.Float](vec *vector.Vector) func(uint64) ( } } +func makeTimeStringIntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) { + param := vector.GenerateFunctionStrParameter(vec) + isBinary := vec.GetIsBin() + return func(i uint64) (int64, bool) { + value, null := param.GetStrValue(i) + if null { + return 0, true + } + if isBinary { + if len(value) > 8 { + return 0, false + } + var result uint64 + for _, b := range value { + result = result<<8 | uint64(b) + } + return int64(result), false + } + result, _ := parseLeadingInteger(strings.TrimSpace(functionUtil.QuickBytesToStr(value))) + return result, false + } +} + // MakeTime: MAKETIME(hour, minute, second) - Returns a time value calculated from the hour, minute, and second arguments. func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *process.Process, length int, selectList *FunctionSelectList) error { rs := vector.MustFunctionResult[types.Time](result) @@ -7366,7 +7389,9 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr var getMinuteValue func(uint64) (int64, bool) var getSecondValue func(uint64) (int64, uint32, bool) - if getter, ok := makeTimeIntegerGetter(ivecs[0]); ok { + if ivecs[0].GetType().Oid.IsMySQLString() { + getHourValue = makeTimeStringIntegerGetter(ivecs[0]) + } else if getter, ok := makeTimeIntegerGetter(ivecs[0]); ok { getHourValue = getter } else { var getFloat func(uint64) (float64, bool) @@ -7394,7 +7419,9 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr } } - if getter, ok := makeTimeIntegerGetter(ivecs[1]); ok { + if ivecs[1].GetType().Oid.IsMySQLString() { + getMinuteValue = makeTimeStringIntegerGetter(ivecs[1]) + } else if getter, ok := makeTimeIntegerGetter(ivecs[1]); ok { getMinuteValue = getter } else { var getFloat func(uint64) (float64, bool) diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index 0a43a72cbde5f..7c0b253ac845e 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9676,6 +9676,55 @@ func TestMakeTimeFloatMinuteRange(t *testing.T) { require.True(t, s, "MAKETIME float minute range failed: %s", info) } +func TestMakeTimeStringHourMinuteSemantics(t *testing.T) { + proc := testutil.NewProcess(t) + + tests := []struct { + name string + inputs []FunctionTestInput + expect FunctionTestResult + }{ + { + name: "string hour and minute truncate while fractional second is preserved", + inputs: []FunctionTestInput{ + NewFunctionTestInput(types.T_varchar.ToType(), []string{"12.7"}, []bool{false}), + NewFunctionTestInput(types.T_varchar.ToType(), []string{"15.8"}, []bool{false}), + NewFunctionTestInput(types.T_float64.ToTypeWithScale(6), []float64{56.789012}, []bool{false}), + }, + expect: NewFunctionTestResult(types.T_time.ToTypeWithScale(6), false, + []types.Time{types.TimeFromClock(false, 12, 15, 56, 789012)}, []bool{false}), + }, + { + name: "only string hour truncates", + inputs: []FunctionTestInput{ + NewFunctionTestInput(types.T_varchar.ToType(), []string{"12.7"}, []bool{false}), + NewFunctionTestInput(types.T_float64.ToTypeWithScale(1), []float64{15.8}, []bool{false}), + NewFunctionTestInput(types.T_float64.ToTypeWithScale(1), []float64{30.9}, []bool{false}), + }, + expect: NewFunctionTestResult(types.T_time.ToTypeWithScale(1), false, + []types.Time{types.TimeFromClock(false, 12, 16, 30, 900000)}, []bool{false}), + }, + { + name: "only string minute truncates", + inputs: []FunctionTestInput{ + NewFunctionTestInput(types.T_float64.ToTypeWithScale(1), []float64{12.7}, []bool{false}), + NewFunctionTestInput(types.T_varchar.ToType(), []string{"15.8"}, []bool{false}), + NewFunctionTestInput(types.T_float64.ToTypeWithScale(1), []float64{30.9}, []bool{false}), + }, + expect: NewFunctionTestResult(types.T_time.ToTypeWithScale(1), false, + []types.Time{types.TimeFromClock(false, 13, 15, 30, 900000)}, []bool{false}), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fcTC := NewFunctionTestCase(proc, test.inputs, test.expect, MakeTime) + s, info := fcTC.Run() + require.True(t, s, "MAKETIME string source semantics failed: %s", info) + }) + } +} + func TestMakeTimeIntegerSecondRange(t *testing.T) { proc := testutil.NewProcess(t) expected := NewFunctionTestResult(types.T_time.ToType(), false, diff --git a/pkg/sql/plan/function/function_test.go b/pkg/sql/plan/function/function_test.go index 9225842a63b34..936ec9df5c008 100644 --- a/pkg/sql/plan/function/function_test.go +++ b/pkg/sql/plan/function/function_test.go @@ -401,6 +401,73 @@ func TestMakeTimeStringSecondUsesFloatOverload(t *testing.T) { require.Equal(t, types.T_time.ToTypeWithScale(6), result.retType) } +func TestMakeTimeStringArgumentTargets(t *testing.T) { + proc := testutil.NewProcess(t) + defaultFloat := types.T_float64.ToType() + defaultFloat.Scale = -1 + scaledFloat := types.T_float64.ToTypeWithScale(1) + + tests := []struct { + name string + inputs []types.Type + overloadArgs []types.T + needCast bool + targets []types.Type + returnType types.Type + }{ + { + name: "varchar hour and minute with double second", + inputs: []types.Type{ + types.T_varchar.ToType(), types.T_varchar.ToType(), defaultFloat, + }, + overloadArgs: []types.T{types.T_varchar, types.T_varchar, types.T_float64}, + returnType: types.T_time.ToTypeWithScale(6), + }, + { + name: "all varchar", + inputs: []types.Type{ + types.T_varchar.ToType(), types.T_varchar.ToType(), types.T_varchar.ToType(), + }, + overloadArgs: []types.T{types.T_varchar, types.T_varchar, types.T_float64}, + needCast: true, + targets: []types.Type{ + types.T_varchar.ToType(), types.T_varchar.ToType(), defaultFloat, + }, + returnType: types.T_time.ToTypeWithScale(6), + }, + { + name: "only hour is varchar", + inputs: []types.Type{ + types.T_varchar.ToType(), scaledFloat, scaledFloat, + }, + overloadArgs: []types.T{types.T_varchar, types.T_float64, types.T_float64}, + returnType: types.T_time.ToTypeWithScale(1), + }, + { + name: "only minute is varchar", + inputs: []types.Type{ + scaledFloat, types.T_varchar.ToType(), scaledFloat, + }, + overloadArgs: []types.T{types.T_float64, types.T_varchar, types.T_float64}, + returnType: types.T_time.ToTypeWithScale(1), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := GetFunctionByName(proc.Ctx, "maketime", test.inputs) + require.NoError(t, err) + require.Equal(t, test.needCast, result.needCast) + require.Equal(t, test.targets, result.targetTypes) + require.Equal(t, test.returnType, result.retType) + + selected, err := GetFunctionById(proc.Ctx, result.GetEncodedOverloadID()) + require.NoError(t, err) + require.Equal(t, test.overloadArgs, selected.args) + }) + } +} + func TestGetFunctionByNameAESDecryptReturnsBlob(t *testing.T) { proc := testutil.NewProcess(t) tests := []struct { diff --git a/pkg/sql/plan/function/list_builtIn.go b/pkg/sql/plan/function/list_builtIn.go index 930ca05ff0f71..2d472e43adea0 100644 --- a/pkg/sql/plan/function/list_builtIn.go +++ b/pkg/sql/plan/function/list_builtIn.go @@ -10748,22 +10748,37 @@ func makeTimeReturnType(parameters []types.Type) types.Type { } func makeTimeCheck(overloads []overload, inputs []types.Type) checkResult { - if len(inputs) == 3 && inputs[2].Oid.IsMySQLString() { - for i, ov := range overloads { - if len(ov.args) != 3 || ov.args[0] != types.T_float64 || ov.args[1] != types.T_float64 || ov.args[2] != types.T_float64 { - continue - } - if status, _ := tryToMatch(inputs, ov.args); status != matchFailed { - targets := make([]types.Type, len(inputs)) - for j := range targets { - targets[j] = ov.args[j].ToType() - SetTargetScaleFromSource(&inputs[j], &targets[j]) - } - return newCheckResultWithCast(i, targets) - } + if len(inputs) != 3 || (!inputs[0].Oid.IsMySQLString() && !inputs[1].Oid.IsMySQLString() && !inputs[2].Oid.IsMySQLString()) { + return fixedTypeMatch(overloads, inputs) + } + + targetOids := []types.T{types.T_float64, types.T_float64, types.T_float64} + if inputs[0].Oid.IsMySQLString() { + targetOids[0] = types.T_varchar + } + if inputs[1].Oid.IsMySQLString() { + targetOids[1] = types.T_varchar + } + status, _ := tryToMatch(inputs, targetOids) + if status == matchFailed { + return fixedTypeMatch(overloads, inputs) + } + + for i, ov := range overloads { + if len(ov.args) != len(targetOids) || ov.args[0] != targetOids[0] || ov.args[1] != targetOids[1] || ov.args[2] != targetOids[2] { + continue } + if status == matchDirectly { + return newCheckResultWithSuccess(i) + } + targets := make([]types.Type, len(inputs)) + for j := range targets { + targets[j] = targetOids[j].ToType() + SetTargetScaleFromSource(&inputs[j], &targets[j]) + } + return newCheckResultWithCast(i, targets) } - return fixedTypeMatch(overloads, inputs) + return newCheckResultWithFailure(failedFunctionParametersWrong) } var supportedControlBuiltIns = []FuncNew{ @@ -11258,6 +11273,30 @@ var supportedControlBuiltIns = []FuncNew{ return MakeTime }, }, + { + overloadId: 5, + args: []types.T{types.T_varchar, types.T_float64, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { + return MakeTime + }, + }, + { + overloadId: 6, + args: []types.T{types.T_float64, types.T_varchar, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { + return MakeTime + }, + }, + { + overloadId: 7, + args: []types.T{types.T_varchar, types.T_varchar, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { + return MakeTime + }, + }, }, }, diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index 9518356754a51..eedf7ea8373dc 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -61,6 +61,15 @@ fractional_seconds SELECT CAST(MAKETIME(12, 34, '56.789012') AS VARCHAR) AS string_fractional_seconds; string_fractional_seconds 12:34:56.789012 +SELECT CAST(MAKETIME('12.7', '15.8', '30.9') AS VARCHAR) AS all_string_values; +all_string_values +12:15:30.900000 +SELECT CAST(MAKETIME('12.7', 15.8, 30.9) AS VARCHAR) AS string_hour_float_minute; +string_hour_float_minute +12:16:30.9 +SELECT CAST(MAKETIME(12.7, '15.8', 30.9) AS VARCHAR) AS float_hour_string_minute; +float_hour_string_minute +13:15:30.9 SELECT CAST(MAKETIME(-12, 34, 56.789012) AS VARCHAR) AS negative_fractional; negative_fractional -12:34:56.789012 @@ -85,6 +94,12 @@ SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS varchar_column_microseconds FROM ma varchar_column_microseconds 12:34:56.789012 DROP TABLE maketime_varchar_seconds; +CREATE TABLE maketime_varchar_parts_double_second(h VARCHAR(20), m VARCHAR(20), s DOUBLE); +INSERT INTO maketime_varchar_parts_double_second VALUES ('12', '34', 56.789012); +SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS varchar_parts_double_second FROM maketime_varchar_parts_double_second; +varchar_parts_double_second +12:34:56.789012 +DROP TABLE maketime_varchar_parts_double_second; CREATE TABLE t1(h INT, m INT, s INT); INSERT INTO t1 VALUES (12, 15, 30), (0, 0, 0), (23, 59, 59); SELECT MAKETIME(h, m, s) AS time_value FROM t1; diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index 01e600beb5789..2c771c72942d5 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -33,6 +33,9 @@ SELECT MAKETIME(NULL, NULL, NULL) AS null_all; SELECT CAST(MAKETIME(12.7, 15.8, 30.9) AS VARCHAR) AS float_values; SELECT CAST(MAKETIME(12, 34, 56.789012) AS VARCHAR) AS fractional_seconds; SELECT CAST(MAKETIME(12, 34, '56.789012') AS VARCHAR) AS string_fractional_seconds; +SELECT CAST(MAKETIME('12.7', '15.8', '30.9') AS VARCHAR) AS all_string_values; +SELECT CAST(MAKETIME('12.7', 15.8, 30.9) AS VARCHAR) AS string_hour_float_minute; +SELECT CAST(MAKETIME(12.7, '15.8', 30.9) AS VARCHAR) AS float_hour_string_minute; SELECT CAST(MAKETIME(-12, 34, 56.789012) AS VARCHAR) AS negative_fractional; SELECT CAST(MAKETIME(12, 59, 59.9999996) AS VARCHAR) AS fractional_carry; SELECT CAST(MAKETIME(838, 59, 59.9999996) AS VARCHAR) AS positive_rounded_endpoint; @@ -50,6 +53,12 @@ INSERT INTO maketime_varchar_seconds VALUES (12, 34, '56.789012'); SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS varchar_column_microseconds FROM maketime_varchar_seconds; DROP TABLE maketime_varchar_seconds; +# VARCHAR hour/minute retain val_int semantics with a DOUBLE second +CREATE TABLE maketime_varchar_parts_double_second(h VARCHAR(20), m VARCHAR(20), s DOUBLE); +INSERT INTO maketime_varchar_parts_double_second VALUES ('12', '34', 56.789012); +SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS varchar_parts_double_second FROM maketime_varchar_parts_double_second; +DROP TABLE maketime_varchar_parts_double_second; + # Table usage CREATE TABLE t1(h INT, m INT, s INT); INSERT INTO t1 VALUES (12, 15, 30), (0, 0, 0), (23, 59, 59); From 3649ba726c23c6ca04a774718e330e65b4681521 Mon Sep 17 00:00:00 2001 From: VioletQwQ-0 <1659648118@qq.com> Date: Mon, 20 Jul 2026 12:45:36 +0800 Subject: [PATCH 22/30] fix: close remaining time function compatibility gaps --- pkg/sql/plan/function/func_binary.go | 108 +++++++++++++++++- pkg/sql/plan/function/func_binary_test.go | 52 ++++++++- pkg/sql/plan/function/function_test.go | 14 ++- pkg/sql/plan/function/list_builtIn.go | 58 +++++++++- .../function/func_datetime_maketime.result | 18 +++ .../function/func_datetime_maketime.test | 10 +- .../function/func_datetime_time_format.result | 6 + .../function/func_datetime_time_format.test | 3 +- 8 files changed, 248 insertions(+), 21 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index c424ebb2a5e4c..396c2edb01753 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -4019,11 +4019,12 @@ func TimeFormat(ivecs []*vector.Vector, result vector.FunctionResultWrapper, pro times := vector.GenerateFunctionFixedTypeParameter[types.Time](ivecs[0]) formats := vector.GenerateFunctionStrParameter(ivecs[1]) fmt, null2 := formats.GetStrValue(0) + emptyFormat := len(fmt) == 0 var buf bytes.Buffer for i := uint64(0); i < uint64(length); i++ { t, null1 := times.GetValue(i) - if null1 || null2 { + if null1 || null2 || emptyFormat { if err = rs.AppendBytes(nil, true); err != nil { return err } @@ -7381,6 +7382,103 @@ func makeTimeStringIntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) } } +func makeTimeExactSecond(value string) (int64, uint32, bool) { + const maxExactSecondDigits = 4096 + + value = strings.TrimSpace(value) + if len(value) == 0 { + return 0, 0, true + } + + end := 0 + if value[end] == '+' || value[end] == '-' { + end++ + } + digits := 0 + for end < len(value) && value[end] >= '0' && value[end] <= '9' { + end++ + digits++ + } + if end < len(value) && value[end] == '.' { + end++ + for end < len(value) && value[end] >= '0' && value[end] <= '9' { + end++ + digits++ + } + } + if digits == 0 { + return 0, 0, true + } + + if end < len(value) && (value[end] == 'e' || value[end] == 'E') { + exponentStart := end + end++ + if end < len(value) && (value[end] == '+' || value[end] == '-') { + end++ + } + exponentDigits := end + exponentMagnitude := 0 + for end < len(value) && value[end] >= '0' && value[end] <= '9' { + if exponentMagnitude <= maxExactSecondDigits { + exponentMagnitude = exponentMagnitude*10 + int(value[end]-'0') + } + end++ + } + if end == exponentDigits { + end = exponentStart + } else if exponentMagnitude > maxExactSecondDigits { + return 0, 0, true + } + } + if end > maxExactSecondDigits { + return 0, 0, true + } + + second, ok := new(big.Rat).SetString(value[:end]) + if !ok || second.Sign() < 0 || second.Cmp(big.NewRat(60, 1)) >= 0 { + return 0, 0, true + } + + scaledNumerator := new(big.Int).Mul(second.Num(), big.NewInt(types.MicroSecsPerSec)) + totalMicroseconds, remainder := new(big.Int), new(big.Int) + totalMicroseconds.QuoRem(scaledNumerator, second.Denom(), remainder) + twiceRemainder := new(big.Int).Lsh(remainder, 1) + if twiceRemainder.Cmp(second.Denom()) >= 0 { + totalMicroseconds.Add(totalMicroseconds, big.NewInt(1)) + } + if !totalMicroseconds.IsInt64() { + return 0, 0, true + } + + total := totalMicroseconds.Int64() + return total / types.MicroSecsPerSec, uint32(total % types.MicroSecsPerSec), false +} + +func makeTimeStringSecondGetter(vec *vector.Vector) func(uint64) (int64, uint32, bool) { + param := vector.GenerateFunctionStrParameter(vec) + isBinary := vec.GetIsBin() + return func(i uint64) (int64, uint32, bool) { + value, null := param.GetStrValue(i) + if null { + return 0, 0, true + } + if isBinary { + if len(value) > 8 { + return 0, 0, true + } + var result uint64 + for _, b := range value { + result = result<<8 | uint64(b) + } + if result > math.MaxInt64 { + return 0, 0, true + } + return makeTimeIntegerSecond(int64(result), false) + } + return makeTimeExactSecond(functionUtil.QuickBytesToStr(value)) + } +} + // MakeTime: MAKETIME(hour, minute, second) - Returns a time value calculated from the hour, minute, and second arguments. func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *process.Process, length int, selectList *FunctionSelectList) error { rs := vector.MustFunctionResult[types.Time](result) @@ -7408,7 +7506,7 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr if null || math.IsNaN(value) || math.IsInf(value, 0) { return 0, true } - rounded := math.RoundToEven(value) + rounded := math.Round(value) if rounded >= float64(math.MaxInt64) { return math.MaxInt64, false } @@ -7438,7 +7536,7 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr if null || math.IsNaN(value) || math.IsInf(value, 0) { return 0, true } - rounded := math.RoundToEven(value) + rounded := math.Round(value) if rounded < 0 || rounded >= 60 { return 0, true } @@ -7446,7 +7544,9 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr } } - if getter, ok := makeTimeIntegerGetter(ivecs[2]); ok { + if ivecs[2].GetType().Oid.IsMySQLString() { + getSecondValue = makeTimeStringSecondGetter(ivecs[2]) + } else if getter, ok := makeTimeIntegerGetter(ivecs[2]); ok { getSecondValue = func(i uint64) (int64, uint32, bool) { value, null := getter(i) return makeTimeIntegerSecond(value, null) diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index 7c0b253ac845e..68b3600373687 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9439,6 +9439,18 @@ func initTimeFormatTestCase() []tcTemp { []string{"-elapsed=123:45:06"}, []bool{false}), }, + { + info: "test time_format - empty format returns null", + inputs: []FunctionTestInput{ + NewFunctionTestInput(types.T_time.ToType(), + []types.Time{t1, t6}, + []bool{false, false}), + NewFunctionTestConstInput(types.T_varchar.ToType(), []string{""}, []bool{false}), + }, + expect: NewFunctionTestResult(types.T_varchar.ToType(), false, + []string{"", ""}, + []bool{true, true}), + }, { info: "test time_format - %h:%i:%s %p", inputs: []FunctionTestInput{ @@ -9630,9 +9642,9 @@ func TestMakeTimeFloatHourRounding(t *testing.T) { NewFunctionTestResult(types.T_time.ToType(), false, []types.Time{ types.TimeFromClock(false, 13, 0, 0, 0), - types.TimeFromClock(false, 12, 0, 0, 0), + types.TimeFromClock(false, 13, 0, 0, 0), types.TimeFromClock(false, 14, 0, 0, 0), - types.TimeFromClock(true, 12, 0, 0, 0), + types.TimeFromClock(true, 13, 0, 0, 0), types.TimeFromClock(true, 14, 0, 0, 0), types.TimeFromClock(false, 838, 59, 59, 0), }, @@ -9660,22 +9672,52 @@ func TestMakeTimeFloatMinuteRange(t *testing.T) { NewFunctionTestResult(types.T_time.ToType(), false, []types.Time{ types.TimeFromClock(false, 12, 16, 0, 0), - types.TimeFromClock(false, 12, 58, 0, 0), + types.TimeFromClock(false, 12, 59, 0, 0), + 0, 0, 0, - types.TimeFromClock(false, 12, 0, 0, 0), 0, 0, 0, 0, }, - []bool{false, false, true, true, false, true, true, true, true}), + []bool{false, false, true, true, true, true, true, true, true}), MakeTime) s, info := fcTC.Run() require.True(t, s, "MAKETIME float minute range failed: %s", info) } +func TestMakeTimeExactStringSecondRounding(t *testing.T) { + proc := testutil.NewProcess(t) + fcTC := NewFunctionTestCase(proc, + []FunctionTestInput{ + NewFunctionTestInput(types.T_int64.ToType(), []int64{12, 12, 12, 12}, []bool{false, false, false, false}), + NewFunctionTestInput(types.T_int64.ToType(), []int64{59, 59, 59, 0}, []bool{false, false, false, false}), + NewFunctionTestInput(types.T_varchar.ToType(), []string{ + "59.99999949999999999", + "59.9999995", + "59.99999950000000001", + "5.9e1", + }, []bool{false, false, false, false}), + }, + NewFunctionTestResult(types.T_time.ToTypeWithScale(6), false, + []types.Time{ + types.TimeFromClock(false, 12, 59, 59, 999999), + types.TimeFromClock(false, 13, 0, 0, 0), + types.TimeFromClock(false, 13, 0, 0, 0), + types.TimeFromClock(false, 12, 0, 59, 0), + }, + []bool{false, false, false, false}), + MakeTime) + + s, info := fcTC.Run() + require.True(t, s, "MAKETIME exact string-second rounding failed: %s", info) + + _, _, null := makeTimeExactSecond("1e999999999") + require.True(t, null, "MAKETIME must reject an unbounded exponent without allocating it") +} + func TestMakeTimeStringHourMinuteSemantics(t *testing.T) { proc := testutil.NewProcess(t) diff --git a/pkg/sql/plan/function/function_test.go b/pkg/sql/plan/function/function_test.go index 936ec9df5c008..884c0d97bd08d 100644 --- a/pkg/sql/plan/function/function_test.go +++ b/pkg/sql/plan/function/function_test.go @@ -372,6 +372,8 @@ func TestMakeTimeReturnScale(t *testing.T) { }) require.NoError(t, err) require.True(t, fractionalResult.needCast) + require.Equal(t, types.T_varchar, fractionalResult.targetTypes[2].Oid) + require.Equal(t, int32(6), fractionalResult.targetTypes[2].Scale) require.Equal(t, types.T_time.ToTypeWithScale(6), fractionalResult.retType) defaultFloatResult, err := GetFunctionByName(proc.Ctx, "maketime", []types.Type{ @@ -383,7 +385,7 @@ func TestMakeTimeReturnScale(t *testing.T) { require.Equal(t, types.T_time.ToTypeWithScale(6), defaultFloatResult.retType) } -func TestMakeTimeStringSecondUsesFloatOverload(t *testing.T) { +func TestMakeTimeStringSecondUsesExactOverload(t *testing.T) { proc := testutil.NewProcess(t) result, err := GetFunctionByName(proc.Ctx, "maketime", []types.Type{ @@ -394,9 +396,9 @@ func TestMakeTimeStringSecondUsesFloatOverload(t *testing.T) { require.NoError(t, err) require.True(t, result.needCast) require.Len(t, result.targetTypes, 3) - for _, target := range result.targetTypes { - require.Equal(t, types.T_float64, target.Oid) - } + require.Equal(t, types.T_float64, result.targetTypes[0].Oid) + require.Equal(t, types.T_float64, result.targetTypes[1].Oid) + require.Equal(t, types.T_varchar, result.targetTypes[2].Oid) require.Equal(t, int32(-1), result.targetTypes[2].Scale) require.Equal(t, types.T_time.ToTypeWithScale(6), result.retType) } @@ -428,10 +430,10 @@ func TestMakeTimeStringArgumentTargets(t *testing.T) { inputs: []types.Type{ types.T_varchar.ToType(), types.T_varchar.ToType(), types.T_varchar.ToType(), }, - overloadArgs: []types.T{types.T_varchar, types.T_varchar, types.T_float64}, + overloadArgs: []types.T{types.T_varchar, types.T_varchar, types.T_varchar}, needCast: true, targets: []types.Type{ - types.T_varchar.ToType(), types.T_varchar.ToType(), defaultFloat, + types.T_varchar.ToType(), types.T_varchar.ToType(), types.T_varchar.ToTypeWithScale(-1), }, returnType: types.T_time.ToTypeWithScale(6), }, diff --git a/pkg/sql/plan/function/list_builtIn.go b/pkg/sql/plan/function/list_builtIn.go index ada28beda3f64..6a68e7375184f 100644 --- a/pkg/sql/plan/function/list_builtIn.go +++ b/pkg/sql/plan/function/list_builtIn.go @@ -10788,7 +10788,11 @@ func makeTimeReturnType(parameters []types.Type) types.Type { } func makeTimeCheck(overloads []overload, inputs []types.Type) checkResult { - if len(inputs) != 3 || (!inputs[0].Oid.IsMySQLString() && !inputs[1].Oid.IsMySQLString() && !inputs[2].Oid.IsMySQLString()) { + if len(inputs) != 3 { + return newCheckResultWithFailure(failedFunctionParametersWrong) + } + exactSecond := inputs[2].Oid.IsMySQLString() || inputs[2].Oid.IsDecimal() + if !inputs[0].Oid.IsMySQLString() && !inputs[1].Oid.IsMySQLString() && !exactSecond { return fixedTypeMatch(overloads, inputs) } @@ -10799,6 +10803,9 @@ func makeTimeCheck(overloads []overload, inputs []types.Type) checkResult { if inputs[1].Oid.IsMySQLString() { targetOids[1] = types.T_varchar } + if exactSecond { + targetOids[2] = types.T_varchar + } status, _ := tryToMatch(inputs, targetOids) if status == matchFailed { return fixedTypeMatch(overloads, inputs) @@ -10808,13 +10815,24 @@ func makeTimeCheck(overloads []overload, inputs []types.Type) checkResult { if len(ov.args) != len(targetOids) || ov.args[0] != targetOids[0] || ov.args[1] != targetOids[1] || ov.args[2] != targetOids[2] { continue } - if status == matchDirectly { + if status == matchDirectly && !exactSecond { return newCheckResultWithSuccess(i) } targets := make([]types.Type, len(inputs)) for j := range targets { - targets[j] = targetOids[j].ToType() - SetTargetScaleFromSource(&inputs[j], &targets[j]) + if inputs[j].Oid == targetOids[j] { + targets[j] = inputs[j] + } else { + targets[j] = targetOids[j].ToType() + SetTargetScaleFromSource(&inputs[j], &targets[j]) + } + } + if exactSecond { + if inputs[2].Oid.IsDecimal() { + targets[2].Scale = inputs[2].Scale + } else { + targets[2].Scale = -1 + } } return newCheckResultWithCast(i, targets) } @@ -11337,6 +11355,38 @@ var supportedControlBuiltIns = []FuncNew{ return MakeTime }, }, + { + overloadId: 8, + args: []types.T{types.T_float64, types.T_float64, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { + return MakeTime + }, + }, + { + overloadId: 9, + args: []types.T{types.T_varchar, types.T_float64, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { + return MakeTime + }, + }, + { + overloadId: 10, + args: []types.T{types.T_float64, types.T_varchar, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { + return MakeTime + }, + }, + { + overloadId: 11, + args: []types.T{types.T_varchar, types.T_varchar, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { + return MakeTime + }, + }, }, }, diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index eedf7ea8373dc..ac8c0d53c45ae 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -55,6 +55,15 @@ null SELECT CAST(MAKETIME(12.7, 15.8, 30.9) AS VARCHAR) AS float_values; float_values 13:16:30.9 +SELECT CAST(MAKETIME(12.5, 58.5, 0) AS VARCHAR) AS positive_half_values; +positive_half_values +13:59:00 +SELECT CAST(MAKETIME(-12.5, 0, 0) AS VARCHAR) AS negative_half_hour; +negative_half_hour +-13:00:00 +SELECT MAKETIME(12, -0.5, 0) AS negative_half_minute; +negative_half_minute +null SELECT CAST(MAKETIME(12, 34, 56.789012) AS VARCHAR) AS fractional_seconds; fractional_seconds 12:34:56.789012 @@ -82,6 +91,15 @@ positive_rounded_endpoint SELECT CAST(MAKETIME(-838, 59, 59.9999996) AS VARCHAR) AS negative_rounded_endpoint; negative_rounded_endpoint -838:59:59.000000 +SELECT CAST(MAKETIME(12, 59, '59.99999949999999999') AS VARCHAR) AS varchar_below_half_microsecond; +varchar_below_half_microsecond +12:59:59.999999 +SELECT CAST(MAKETIME(12, 59, '59.9999995') AS VARCHAR) AS varchar_at_half_microsecond; +varchar_at_half_microsecond +13:00:00.000000 +SELECT CAST(MAKETIME(12, 59, CAST('59.99999949999999999' AS DECIMAL(30,20))) AS VARCHAR) AS decimal_below_half_microsecond; +decimal_below_half_microsecond +12:59:59.999999 CREATE TABLE maketime_double_seconds(h INT, m INT, s DOUBLE); INSERT INTO maketime_double_seconds VALUES (12, 34, 56.789012); SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS double_column_microseconds FROM maketime_double_seconds; diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index 2c771c72942d5..fb57809adeb2d 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -29,8 +29,11 @@ SELECT MAKETIME(12, NULL, 30) AS null_minute; SELECT MAKETIME(12, 15, NULL) AS null_second; SELECT MAKETIME(NULL, NULL, NULL) AS null_all; -# Float hour and minute values round to nearest-even; fractional seconds are preserved +# Float hour and minute values round half away from zero; fractional seconds are preserved SELECT CAST(MAKETIME(12.7, 15.8, 30.9) AS VARCHAR) AS float_values; +SELECT CAST(MAKETIME(12.5, 58.5, 0) AS VARCHAR) AS positive_half_values; +SELECT CAST(MAKETIME(-12.5, 0, 0) AS VARCHAR) AS negative_half_hour; +SELECT MAKETIME(12, -0.5, 0) AS negative_half_minute; SELECT CAST(MAKETIME(12, 34, 56.789012) AS VARCHAR) AS fractional_seconds; SELECT CAST(MAKETIME(12, 34, '56.789012') AS VARCHAR) AS string_fractional_seconds; SELECT CAST(MAKETIME('12.7', '15.8', '30.9') AS VARCHAR) AS all_string_values; @@ -41,6 +44,11 @@ SELECT CAST(MAKETIME(12, 59, 59.9999996) AS VARCHAR) AS fractional_carry; SELECT CAST(MAKETIME(838, 59, 59.9999996) AS VARCHAR) AS positive_rounded_endpoint; SELECT CAST(MAKETIME(-838, 59, 59.9999996) AS VARCHAR) AS negative_rounded_endpoint; +# Exact VARCHAR/DECIMAL seconds do not cross the microsecond boundary through FLOAT64 +SELECT CAST(MAKETIME(12, 59, '59.99999949999999999') AS VARCHAR) AS varchar_below_half_microsecond; +SELECT CAST(MAKETIME(12, 59, '59.9999995') AS VARCHAR) AS varchar_at_half_microsecond; +SELECT CAST(MAKETIME(12, 59, CAST('59.99999949999999999' AS DECIMAL(30,20))) AS VARCHAR) AS decimal_below_half_microsecond; + # Default DOUBLE column retains microseconds CREATE TABLE maketime_double_seconds(h INT, m INT, s DOUBLE); INSERT INTO maketime_double_seconds VALUES (12, 34, 56.789012); diff --git a/test/distributed/cases/function/func_datetime_time_format.result b/test/distributed/cases/function/func_datetime_time_format.result index 332bc6baa38aa..9d085e457fff0 100644 --- a/test/distributed/cases/function/func_datetime_time_format.result +++ b/test/distributed/cases/function/func_datetime_time_format.result @@ -76,6 +76,12 @@ null SELECT TIME_FORMAT('15:30:45', NULL) AS null_format; null_format null +SELECT TIME_FORMAT('15:30:45', '') AS empty_positive_format; +empty_positive_format +null +SELECT TIME_FORMAT('-15:30:45', '') AS empty_negative_format; +empty_negative_format +null CREATE TABLE t1(t TIME); INSERT INTO t1 VALUES ('15:30:45'), ('00:00:00'), ('23:59:59'), ('12:34:56'); SELECT t, TIME_FORMAT(t, '%H:%i:%s') AS formatted FROM t1; diff --git a/test/distributed/cases/function/func_datetime_time_format.test b/test/distributed/cases/function/func_datetime_time_format.test index a45f894d2e59e..98e99771486fe 100644 --- a/test/distributed/cases/function/func_datetime_time_format.test +++ b/test/distributed/cases/function/func_datetime_time_format.test @@ -40,6 +40,8 @@ SELECT TIME_FORMAT('15:30:45', '%s') AS second_alt; # NULL handling SELECT TIME_FORMAT(NULL, '%H:%i:%s') AS null_time; SELECT TIME_FORMAT('15:30:45', NULL) AS null_format; +SELECT TIME_FORMAT('15:30:45', '') AS empty_positive_format; +SELECT TIME_FORMAT('-15:30:45', '') AS empty_negative_format; # Table usage CREATE TABLE t1(t TIME); @@ -56,4 +58,3 @@ DROP TABLE t1; # Complex format strings SELECT TIME_FORMAT('15:30:45.123456', '%H:%i:%s.%f') AS complex_format; SELECT TIME_FORMAT('15:30:45', 'Time is %H:%i:%s') AS with_text; - From 3f329d10fa9ab23bd8ee96c895556fb7edea1fbb Mon Sep 17 00:00:00 2001 From: VioletQwQ-0 <1659648118@qq.com> Date: Mon, 20 Jul 2026 14:27:46 +0800 Subject: [PATCH 23/30] fix(function): preserve binary MAKETIME semantics --- pkg/sql/plan/function/function_test.go | 21 +++++++++++++++++++ pkg/sql/plan/function/list_builtIn.go | 19 +++++++++++++---- .../function/func_datetime_maketime.result | 6 ++++++ .../function/func_datetime_maketime.test | 4 ++++ 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/pkg/sql/plan/function/function_test.go b/pkg/sql/plan/function/function_test.go index 884c0d97bd08d..d12670981c16a 100644 --- a/pkg/sql/plan/function/function_test.go +++ b/pkg/sql/plan/function/function_test.go @@ -470,6 +470,27 @@ func TestMakeTimeStringArgumentTargets(t *testing.T) { } } +func TestMakeTimeBinaryArgumentsUseNumericOverloads(t *testing.T) { + proc := testutil.NewProcess(t) + binaryTypes := []types.T{types.T_binary, types.T_varbinary, types.T_blob} + + for _, binaryType := range binaryTypes { + for position := range 3 { + inputs := []types.Type{ + types.T_int64.ToType(), + types.T_int64.ToType(), + types.T_int64.ToType(), + } + inputs[position] = binaryType.ToType() + + result, err := GetFunctionByName(proc.Ctx, "maketime", inputs) + require.NoError(t, err) + require.True(t, result.needCast) + require.Equal(t, types.T_int64, result.targetTypes[position].Oid) + } + } +} + func TestGetFunctionByNameAESDecryptReturnsBlob(t *testing.T) { proc := testutil.NewProcess(t) tests := []struct { diff --git a/pkg/sql/plan/function/list_builtIn.go b/pkg/sql/plan/function/list_builtIn.go index 6a68e7375184f..792338d54e954 100644 --- a/pkg/sql/plan/function/list_builtIn.go +++ b/pkg/sql/plan/function/list_builtIn.go @@ -10787,20 +10787,31 @@ func makeTimeReturnType(parameters []types.Type) types.Type { return types.T_time.ToTypeWithScale(scale) } +func isMakeTimeTextType(oid types.T) bool { + // Binary inputs must take the numeric cast path so hex/bit literal byte + // semantics are consumed before function-expression evaluation clears IsBin. + switch oid { + case types.T_binary, types.T_varbinary, types.T_blob: + return false + default: + return oid.IsMySQLString() + } +} + func makeTimeCheck(overloads []overload, inputs []types.Type) checkResult { if len(inputs) != 3 { return newCheckResultWithFailure(failedFunctionParametersWrong) } - exactSecond := inputs[2].Oid.IsMySQLString() || inputs[2].Oid.IsDecimal() - if !inputs[0].Oid.IsMySQLString() && !inputs[1].Oid.IsMySQLString() && !exactSecond { + exactSecond := isMakeTimeTextType(inputs[2].Oid) || inputs[2].Oid.IsDecimal() + if !isMakeTimeTextType(inputs[0].Oid) && !isMakeTimeTextType(inputs[1].Oid) && !exactSecond { return fixedTypeMatch(overloads, inputs) } targetOids := []types.T{types.T_float64, types.T_float64, types.T_float64} - if inputs[0].Oid.IsMySQLString() { + if isMakeTimeTextType(inputs[0].Oid) { targetOids[0] = types.T_varchar } - if inputs[1].Oid.IsMySQLString() { + if isMakeTimeTextType(inputs[1].Oid) { targetOids[1] = types.T_varchar } if exactSecond { diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index ac8c0d53c45ae..d69bf1c3d4ecb 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -100,6 +100,12 @@ varchar_at_half_microsecond SELECT CAST(MAKETIME(12, 59, CAST('59.99999949999999999' AS DECIMAL(30,20))) AS VARCHAR) AS decimal_below_half_microsecond; decimal_below_half_microsecond 12:59:59.999999 +SELECT CAST(MAKETIME(X'0102', 0, 0) AS VARCHAR) AS binary_hour; +binary_hour +258:00:00 +SELECT CAST(MAKETIME(12, 0, X'01') AS VARCHAR) AS binary_second; +binary_second +12:00:01 CREATE TABLE maketime_double_seconds(h INT, m INT, s DOUBLE); INSERT INTO maketime_double_seconds VALUES (12, 34, 56.789012); SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS double_column_microseconds FROM maketime_double_seconds; diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index fb57809adeb2d..13b3d51ceeb0e 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -49,6 +49,10 @@ SELECT CAST(MAKETIME(12, 59, '59.99999949999999999') AS VARCHAR) AS varchar_belo SELECT CAST(MAKETIME(12, 59, '59.9999995') AS VARCHAR) AS varchar_at_half_microsecond; SELECT CAST(MAKETIME(12, 59, CAST('59.99999949999999999' AS DECIMAL(30,20))) AS VARCHAR) AS decimal_below_half_microsecond; +# Binary literals retain numeric byte semantics instead of being parsed as text +SELECT CAST(MAKETIME(X'0102', 0, 0) AS VARCHAR) AS binary_hour; +SELECT CAST(MAKETIME(12, 0, X'01') AS VARCHAR) AS binary_second; + # Default DOUBLE column retains microseconds CREATE TABLE maketime_double_seconds(h INT, m INT, s DOUBLE); INSERT INTO maketime_double_seconds VALUES (12, 34, 56.789012); From bb599c6a87a98c414d6ff5fec1761cd69c5ad1af Mon Sep 17 00:00:00 2001 From: VioletQwQ-0 <1659648118@qq.com> Date: Mon, 20 Jul 2026 15:32:09 +0800 Subject: [PATCH 24/30] fix(plan): preserve binary maketime seconds --- pkg/sql/plan/base_binder.go | 19 ++++++++++++++++ pkg/sql/plan/build_expr_test.go | 40 +++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/pkg/sql/plan/base_binder.go b/pkg/sql/plan/base_binder.go index 2c78e0269b479..0947a156f3dfa 100644 --- a/pkg/sql/plan/base_binder.go +++ b/pkg/sql/plan/base_binder.go @@ -2691,6 +2691,16 @@ func BindFuncExprImplByPlanExpr(ctx context.Context, name string, args []*Expr) } } + case "maketime": + // Hex and bit literals are represented as VARCHAR literals carrying + // IsBin. They are integral seconds, so they retain TIME(0) metadata even + // though the VARCHAR seconds overload normally advertises TIME(6). + if len(args) == 3 { + if literal := args[2].GetLit(); literal != nil && literal.IsBin { + returnType.Scale = 0 + } + } + case "timestampadd": // For TIMESTAMPADD with DATE input, check if unit is constant and adjust return type // MySQL behavior: DATE input + date unit → DATE output, DATE input + time unit → DATETIME output @@ -2741,6 +2751,15 @@ func BindFuncExprImplByPlanExpr(ctx context.Context, name string, args []*Expr) } for idx, castType := range argsCastType { if !argsType[idx].Eq(castType) && castType.Oid != types.T_any { + // MAKETIME uses the scale on its VARCHAR seconds target only to + // derive the TIME return scale. Recasting an already-VARCHAR + // argument solely for that metadata clears Literal.IsBin, changing + // X'..'/B'..' from a binary number into ordinary text. + if name == "maketime" && idx == 2 && + argsType[idx].Oid == types.T_varchar && castType.Oid == types.T_varchar && + argsType[idx].Width == castType.Width { + continue + } if argsType[idx].Oid == castType.Oid && castType.Oid.IsDecimal() && argsType[idx].Scale == castType.Scale { continue } diff --git a/pkg/sql/plan/build_expr_test.go b/pkg/sql/plan/build_expr_test.go index 4e80bba5243b5..8b43fe7673e02 100644 --- a/pkg/sql/plan/build_expr_test.go +++ b/pkg/sql/plan/build_expr_test.go @@ -402,6 +402,46 @@ func runOneExprStmt(opt Optimizer, t *testing.T, sql string) (*plan.Plan, error) return pl, nil } +func TestMakeTimeBinaryLiteralBindAndExecute(t *testing.T) { + tests := []struct { + name string + sql string + want string + }{ + {name: "hex hour", sql: "select cast(maketime(X'0102', 0, 0) as varchar)", want: "258:00:00"}, + {name: "hex hour uint64 overflow", sql: "select cast(maketime(X'FFFFFFFFFFFFFFFF', 0, 0) as varchar)", want: "838:59:59"}, + {name: "hex hour wider than uint64", sql: "select cast(maketime(X'FFFFFFFFFFFFFFFFFF', 0, 0) as varchar)", want: "838:59:59"}, + {name: "hex minute", sql: "select cast(maketime(12, X'01', 0) as varchar)", want: "12:01:00"}, + {name: "hex second", sql: "select cast(maketime(12, 0, X'01') as varchar)", want: "12:00:01"}, + {name: "bit second", sql: "select cast(maketime(12, 0, B'00000001') as varchar)", want: "12:00:01"}, + {name: "binary string second", sql: "select cast(maketime(12, 0, cast('01' as binary(2))) as varchar)", want: "12:00:01"}, + {name: "plain strings", sql: "select cast(maketime('12.7', '15.8', '30.9') as varchar)", want: "12:15:30.900000"}, + {name: "decimal second", sql: "select cast(maketime(12, 34, cast('56.789012' as decimal(20, 6))) as varchar)", want: "12:34:56.789012"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mock := NewMockOptimizer(false) + pl, err := runOneExprStmt(mock, t, test.sql) + require.NoError(t, err) + + query := pl.GetQuery() + require.NotNil(t, query) + expr := query.Nodes[1].ProjectList[0] + proc := testutil.NewProc(t) + defer proc.Free() + executor, err := colexec.NewExpressionExecutor(proc, expr) + require.NoError(t, err) + defer executor.Free() + + result, err := executor.Eval(proc, nil, nil) + require.NoError(t, err) + require.False(t, result.GetNulls().Contains(0)) + require.Equal(t, test.want, result.GetStringAt(0)) + }) + } +} + func makeTimeExpr(s string, p int32) *plan.Expr { dt, _ := types.ParseTime(s, 0) return &plan.Expr{ From 50073bfb390881f6120424b45f04a70bb1019be1 Mon Sep 17 00:00:00 2001 From: VioletQwQ-0 <1659648118@qq.com> Date: Mon, 20 Jul 2026 16:03:38 +0800 Subject: [PATCH 25/30] fix(function): clamp binary MAKETIME overflow --- pkg/sql/plan/build_expr_test.go | 17 +++++++++++--- pkg/sql/plan/function/func_binary.go | 27 ++++++++++++++++------- pkg/sql/plan/function/func_binary_test.go | 23 +++++++++++++++++++ 3 files changed, 56 insertions(+), 11 deletions(-) diff --git a/pkg/sql/plan/build_expr_test.go b/pkg/sql/plan/build_expr_test.go index 8b43fe7673e02..03e0454820af1 100644 --- a/pkg/sql/plan/build_expr_test.go +++ b/pkg/sql/plan/build_expr_test.go @@ -404,14 +404,21 @@ func runOneExprStmt(opt Optimizer, t *testing.T, sql string) (*plan.Plan, error) func TestMakeTimeBinaryLiteralBindAndExecute(t *testing.T) { tests := []struct { - name string - sql string - want string + name string + sql string + want string + wantNull bool }{ {name: "hex hour", sql: "select cast(maketime(X'0102', 0, 0) as varchar)", want: "258:00:00"}, + {name: "hex hour empty", sql: "select cast(maketime(X'', 0, 0) as varchar)", want: "00:00:00"}, + {name: "hex hour max int64", sql: "select cast(maketime(X'7FFFFFFFFFFFFFFF', 0, 0) as varchar)", want: "838:59:59"}, + {name: "hex hour max int64 plus one", sql: "select cast(maketime(X'8000000000000000', 0, 0) as varchar)", want: "838:59:59"}, {name: "hex hour uint64 overflow", sql: "select cast(maketime(X'FFFFFFFFFFFFFFFF', 0, 0) as varchar)", want: "838:59:59"}, {name: "hex hour wider than uint64", sql: "select cast(maketime(X'FFFFFFFFFFFFFFFFFF', 0, 0) as varchar)", want: "838:59:59"}, + {name: "hex hour wide leading zeros", sql: "select cast(maketime(X'000000000000000001', 0, 0) as varchar)", want: "01:00:00"}, {name: "hex minute", sql: "select cast(maketime(12, X'01', 0) as varchar)", want: "12:01:00"}, + {name: "hex minute overflow", sql: "select cast(maketime(12, X'FFFFFFFFFFFFFFFFFF', 0) as varchar)", wantNull: true}, + {name: "hex minute wide leading zeros", sql: "select cast(maketime(12, X'000000000000000001', 0) as varchar)", want: "12:01:00"}, {name: "hex second", sql: "select cast(maketime(12, 0, X'01') as varchar)", want: "12:00:01"}, {name: "bit second", sql: "select cast(maketime(12, 0, B'00000001') as varchar)", want: "12:00:01"}, {name: "binary string second", sql: "select cast(maketime(12, 0, cast('01' as binary(2))) as varchar)", want: "12:00:01"}, @@ -436,6 +443,10 @@ func TestMakeTimeBinaryLiteralBindAndExecute(t *testing.T) { result, err := executor.Eval(proc, nil, nil) require.NoError(t, err) + if test.wantNull { + require.True(t, result.GetNulls().Contains(0)) + return + } require.False(t, result.GetNulls().Contains(0)) require.Equal(t, test.want, result.GetStringAt(0)) }) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 396c2edb01753..85a88f1f2b9ce 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7351,6 +7351,24 @@ func makeTimeIntegerGetter(vec *vector.Vector) (func(uint64) (int64, bool), bool } } +func makeTimeBinaryInteger(value []byte) int64 { + for len(value) > 0 && value[0] == 0 { + value = value[1:] + } + if len(value) > 8 { + return math.MaxInt64 + } + + var result uint64 + for _, b := range value { + result = result<<8 | uint64(b) + } + if result > math.MaxInt64 { + return math.MaxInt64 + } + return int64(result) +} + func makeTimeFloatGetter[T constraints.Float](vec *vector.Vector) func(uint64) (float64, bool) { param := vector.GenerateFunctionFixedTypeParameter[T](vec) return func(i uint64) (float64, bool) { @@ -7368,14 +7386,7 @@ func makeTimeStringIntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) return 0, true } if isBinary { - if len(value) > 8 { - return 0, false - } - var result uint64 - for _, b := range value { - result = result<<8 | uint64(b) - } - return int64(result), false + return makeTimeBinaryInteger(value), false } result, _ := parseLeadingInteger(strings.TrimSpace(functionUtil.QuickBytesToStr(value))) return result, false diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index 68b3600373687..a75d5294e1089 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9582,6 +9582,29 @@ func TestMakeTimeUnsignedHourOverflow(t *testing.T) { require.True(t, s, "MAKETIME unsigned hour overflow case failed: %s", info) } +func TestMakeTimeBinaryIntegerBoundaries(t *testing.T) { + tests := []struct { + name string + value []byte + want int64 + }{ + {name: "empty", value: nil, want: 0}, + {name: "zero", value: []byte{0}, want: 0}, + {name: "max int64", value: []byte{0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, want: math.MaxInt64}, + {name: "max int64 plus one", value: []byte{0x80, 0, 0, 0, 0, 0, 0, 0}, want: math.MaxInt64}, + {name: "max uint64", value: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, want: math.MaxInt64}, + {name: "wider than uint64", value: []byte{1, 0, 0, 0, 0, 0, 0, 0, 0}, want: math.MaxInt64}, + {name: "wide leading zeros", value: []byte{0, 0, 0, 0, 0, 0, 0, 0, 1}, want: 1}, + {name: "wide leading zero max int64", value: []byte{0, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, want: math.MaxInt64}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.want, makeTimeBinaryInteger(test.value)) + }) + } +} + func TestMakeTimeSignedHourOverflow(t *testing.T) { proc := testutil.NewProcess(t) fcTC := NewFunctionTestCase(proc, From 57b33f78518b0ba2ac1d859a97ba34fd5f613f68 Mon Sep 17 00:00:00 2001 From: VioletQwQ-0 <1659648118@qq.com> Date: Mon, 20 Jul 2026 17:59:39 +0800 Subject: [PATCH 26/30] fix: preserve empty time format result --- pkg/sql/plan/function/func_binary.go | 5 ++--- pkg/sql/plan/function/func_binary_test.go | 4 ++-- .../cases/function/func_datetime_time_format.result | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 85a88f1f2b9ce..23630732bf1c2 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -4019,12 +4019,11 @@ func TimeFormat(ivecs []*vector.Vector, result vector.FunctionResultWrapper, pro times := vector.GenerateFunctionFixedTypeParameter[types.Time](ivecs[0]) formats := vector.GenerateFunctionStrParameter(ivecs[1]) fmt, null2 := formats.GetStrValue(0) - emptyFormat := len(fmt) == 0 var buf bytes.Buffer for i := uint64(0); i < uint64(length); i++ { t, null1 := times.GetValue(i) - if null1 || null2 || emptyFormat { + if null1 || null2 { if err = rs.AppendBytes(nil, true); err != nil { return err } @@ -4045,7 +4044,7 @@ func TimeFormat(ivecs []*vector.Vector, result vector.FunctionResultWrapper, pro // Only supports time-related format specifiers: %H, %h, %I, %i, %k, %l, %S, %s, %f, %p, %r, %T func timeFormat(ctx context.Context, t types.Time, format string, buf *bytes.Buffer) error { hour, minute, sec, msec, isNeg := t.ClockFormat() - if isNeg { + if isNeg && len(format) > 0 { buf.WriteByte('-') } inPatternMatch := false diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index a75d5294e1089..e4a25764039aa 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9440,7 +9440,7 @@ func initTimeFormatTestCase() []tcTemp { []bool{false}), }, { - info: "test time_format - empty format returns null", + info: "test time_format - empty format returns empty string", inputs: []FunctionTestInput{ NewFunctionTestInput(types.T_time.ToType(), []types.Time{t1, t6}, @@ -9449,7 +9449,7 @@ func initTimeFormatTestCase() []tcTemp { }, expect: NewFunctionTestResult(types.T_varchar.ToType(), false, []string{"", ""}, - []bool{true, true}), + []bool{false, false}), }, { info: "test time_format - %h:%i:%s %p", diff --git a/test/distributed/cases/function/func_datetime_time_format.result b/test/distributed/cases/function/func_datetime_time_format.result index 9d085e457fff0..49c35ca4829e2 100644 --- a/test/distributed/cases/function/func_datetime_time_format.result +++ b/test/distributed/cases/function/func_datetime_time_format.result @@ -78,10 +78,10 @@ null_format null SELECT TIME_FORMAT('15:30:45', '') AS empty_positive_format; empty_positive_format -null + SELECT TIME_FORMAT('-15:30:45', '') AS empty_negative_format; empty_negative_format -null + CREATE TABLE t1(t TIME); INSERT INTO t1 VALUES ('15:30:45'), ('00:00:00'), ('23:59:59'), ('12:34:56'); SELECT t, TIME_FORMAT(t, '%H:%i:%s') AS formatted FROM t1; From 8f1ca3ac32c32d15d786208bfb758028bb186953 Mon Sep 17 00:00:00 2001 From: VioletQwQ-0 <1659648118@qq.com> Date: Mon, 20 Jul 2026 21:55:04 +0800 Subject: [PATCH 27/30] fix(function): coerce maketime string and binary seconds --- pkg/sql/plan/build_expr_test.go | 5 +++++ pkg/sql/plan/function/func_binary.go | 16 +++------------- pkg/sql/plan/function/func_binary_test.go | 12 ++++++++---- .../cases/function/func_datetime_maketime.result | 9 +++++++++ .../cases/function/func_datetime_maketime.test | 5 +++++ 5 files changed, 30 insertions(+), 17 deletions(-) diff --git a/pkg/sql/plan/build_expr_test.go b/pkg/sql/plan/build_expr_test.go index 03e0454820af1..87e37f7fcb163 100644 --- a/pkg/sql/plan/build_expr_test.go +++ b/pkg/sql/plan/build_expr_test.go @@ -420,8 +420,13 @@ func TestMakeTimeBinaryLiteralBindAndExecute(t *testing.T) { {name: "hex minute overflow", sql: "select cast(maketime(12, X'FFFFFFFFFFFFFFFFFF', 0) as varchar)", wantNull: true}, {name: "hex minute wide leading zeros", sql: "select cast(maketime(12, X'000000000000000001', 0) as varchar)", want: "12:01:00"}, {name: "hex second", sql: "select cast(maketime(12, 0, X'01') as varchar)", want: "12:00:01"}, + {name: "hex second wide leading zeros", sql: "select cast(maketime(12, 0, X'000000000000000001') as varchar)", want: "12:00:01"}, + {name: "hex second wide leading zero max", sql: "select cast(maketime(12, 0, X'00000000000000003B') as varchar)", want: "12:00:59"}, + {name: "hex second wider overflow", sql: "select cast(maketime(12, 0, X'010000000000000000') as varchar)", wantNull: true}, {name: "bit second", sql: "select cast(maketime(12, 0, B'00000001') as varchar)", want: "12:00:01"}, {name: "binary string second", sql: "select cast(maketime(12, 0, cast('01' as binary(2))) as varchar)", want: "12:00:01"}, + {name: "empty string second coerces to zero", sql: "select cast(maketime(12, 34, '') as varchar)", want: "12:34:00.000000"}, + {name: "nonnumeric string second coerces to zero", sql: "select cast(maketime(12, 34, 'foo') as varchar)", want: "12:34:00.000000"}, {name: "plain strings", sql: "select cast(maketime('12.7', '15.8', '30.9') as varchar)", want: "12:15:30.900000"}, {name: "decimal second", sql: "select cast(maketime(12, 34, cast('56.789012' as decimal(20, 6))) as varchar)", want: "12:34:56.789012"}, } diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 23630732bf1c2..3fed76a61c63c 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7397,7 +7397,7 @@ func makeTimeExactSecond(value string) (int64, uint32, bool) { value = strings.TrimSpace(value) if len(value) == 0 { - return 0, 0, true + return 0, 0, false } end := 0 @@ -7417,7 +7417,7 @@ func makeTimeExactSecond(value string) (int64, uint32, bool) { } } if digits == 0 { - return 0, 0, true + return 0, 0, false } if end < len(value) && (value[end] == 'e' || value[end] == 'E') { @@ -7473,17 +7473,7 @@ func makeTimeStringSecondGetter(vec *vector.Vector) func(uint64) (int64, uint32, return 0, 0, true } if isBinary { - if len(value) > 8 { - return 0, 0, true - } - var result uint64 - for _, b := range value { - result = result<<8 | uint64(b) - } - if result > math.MaxInt64 { - return 0, 0, true - } - return makeTimeIntegerSecond(int64(result), false) + return makeTimeIntegerSecond(makeTimeBinaryInteger(value), false) } return makeTimeExactSecond(functionUtil.QuickBytesToStr(value)) } diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index e4a25764039aa..75356cce4f250 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9715,14 +9715,16 @@ func TestMakeTimeExactStringSecondRounding(t *testing.T) { proc := testutil.NewProcess(t) fcTC := NewFunctionTestCase(proc, []FunctionTestInput{ - NewFunctionTestInput(types.T_int64.ToType(), []int64{12, 12, 12, 12}, []bool{false, false, false, false}), - NewFunctionTestInput(types.T_int64.ToType(), []int64{59, 59, 59, 0}, []bool{false, false, false, false}), + NewFunctionTestInput(types.T_int64.ToType(), []int64{12, 12, 12, 12, 12, 12}, []bool{false, false, false, false, false, false}), + NewFunctionTestInput(types.T_int64.ToType(), []int64{59, 59, 59, 0, 34, 34}, []bool{false, false, false, false, false, false}), NewFunctionTestInput(types.T_varchar.ToType(), []string{ "59.99999949999999999", "59.9999995", "59.99999950000000001", "5.9e1", - }, []bool{false, false, false, false}), + "", + "foo", + }, []bool{false, false, false, false, false, false}), }, NewFunctionTestResult(types.T_time.ToTypeWithScale(6), false, []types.Time{ @@ -9730,8 +9732,10 @@ func TestMakeTimeExactStringSecondRounding(t *testing.T) { types.TimeFromClock(false, 13, 0, 0, 0), types.TimeFromClock(false, 13, 0, 0, 0), types.TimeFromClock(false, 12, 0, 59, 0), + types.TimeFromClock(false, 12, 34, 0, 0), + types.TimeFromClock(false, 12, 34, 0, 0), }, - []bool{false, false, false, false}), + []bool{false, false, false, false, false, false}), MakeTime) s, info := fcTC.Run() diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index d69bf1c3d4ecb..6c2958af750bd 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -106,6 +106,15 @@ binary_hour SELECT CAST(MAKETIME(12, 0, X'01') AS VARCHAR) AS binary_second; binary_second 12:00:01 +SELECT CAST(MAKETIME(12, 0, X'000000000000000001') AS VARCHAR) AS binary_second_wide_leading_zeros; +binary_second_wide_leading_zeros +12:00:01 +SELECT CAST(MAKETIME(12, 34, '') AS VARCHAR) AS empty_string_second; +empty_string_second +12:34:00.000000 +SELECT CAST(MAKETIME(12, 34, 'foo') AS VARCHAR) AS nonnumeric_string_second; +nonnumeric_string_second +12:34:00.000000 CREATE TABLE maketime_double_seconds(h INT, m INT, s DOUBLE); INSERT INTO maketime_double_seconds VALUES (12, 34, 56.789012); SELECT CAST(MAKETIME(h, m, s) AS VARCHAR) AS double_column_microseconds FROM maketime_double_seconds; diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index 13b3d51ceeb0e..dbbbc080883ca 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -52,6 +52,11 @@ SELECT CAST(MAKETIME(12, 59, CAST('59.99999949999999999' AS DECIMAL(30,20))) AS # Binary literals retain numeric byte semantics instead of being parsed as text SELECT CAST(MAKETIME(X'0102', 0, 0) AS VARCHAR) AS binary_hour; SELECT CAST(MAKETIME(12, 0, X'01') AS VARCHAR) AS binary_second; +SELECT CAST(MAKETIME(12, 0, X'000000000000000001') AS VARCHAR) AS binary_second_wide_leading_zeros; + +# Exact VARCHAR seconds use MySQL numeric coercion for empty and nonnumeric text +SELECT CAST(MAKETIME(12, 34, '') AS VARCHAR) AS empty_string_second; +SELECT CAST(MAKETIME(12, 34, 'foo') AS VARCHAR) AS nonnumeric_string_second; # Default DOUBLE column retains microseconds CREATE TABLE maketime_double_seconds(h INT, m INT, s DOUBLE); From 3e1d50ae90d748ed3d6e771c2e13d294e1536916 Mon Sep 17 00:00:00 2001 From: VioletQwQ-0 <1659648118@qq.com> Date: Wed, 22 Jul 2026 15:38:10 +0800 Subject: [PATCH 28/30] fix: preserve decimal maketime rounding precision --- pkg/sql/plan/build_expr_test.go | 37 ++++++++++ pkg/sql/plan/function/func_binary.go | 36 ++++++++++ pkg/sql/plan/function/function_test.go | 30 ++++++++ pkg/sql/plan/function/list_builtIn.go | 68 ++++++++++++++++++- .../function/func_datetime_maketime.result | 3 + .../function/func_datetime_maketime.test | 3 + 6 files changed, 176 insertions(+), 1 deletion(-) diff --git a/pkg/sql/plan/build_expr_test.go b/pkg/sql/plan/build_expr_test.go index 87e37f7fcb163..4a0cfd4086bf9 100644 --- a/pkg/sql/plan/build_expr_test.go +++ b/pkg/sql/plan/build_expr_test.go @@ -15,6 +15,7 @@ package plan import ( + "strings" "testing" "time" @@ -429,6 +430,14 @@ func TestMakeTimeBinaryLiteralBindAndExecute(t *testing.T) { {name: "nonnumeric string second coerces to zero", sql: "select cast(maketime(12, 34, 'foo') as varchar)", want: "12:34:00.000000"}, {name: "plain strings", sql: "select cast(maketime('12.7', '15.8', '30.9') as varchar)", want: "12:15:30.900000"}, {name: "decimal second", sql: "select cast(maketime(12, 34, cast('56.789012' as decimal(20, 6))) as varchar)", want: "12:34:56.789012"}, + {name: "decimal minute below half", sql: "select cast(maketime(12, cast('59.49999999999999999999' as decimal(30, 20)), cast('0' as decimal(2, 1))) as varchar)", want: "12:59:00.0"}, + {name: "decimal minute at half", sql: "select cast(maketime(12, cast('58.5' as decimal(3, 1)), 0) as varchar)", want: "12:59:00"}, + {name: "decimal64 minute below half", sql: "select cast(maketime(12, cast('58.499999999999999' as decimal(17, 15)), 0) as varchar)", want: "12:58:00"}, + {name: "decimal minute rounds out of range", sql: "select maketime(12, cast('59.5' as decimal(3, 1)), 0)", wantNull: true}, + {name: "decimal hour below half", sql: "select cast(maketime(cast('12.49999999999999999999' as decimal(30, 20)), 0, 0) as varchar)", want: "12:00:00"}, + {name: "negative decimal hour at half", sql: "select cast(maketime(cast('-12.5' as decimal(3, 1)), 0, 0) as varchar)", want: "-13:00:00"}, + {name: "decimal hour positive overflow", sql: "select cast(maketime(cast('99999999999999999999999999999999999999' as decimal(38, 0)), 0, 0) as varchar)", want: "838:59:59"}, + {name: "decimal hour negative overflow", sql: "select cast(maketime(cast('-99999999999999999999999999999999999999' as decimal(38, 0)), 0, 0) as varchar)", want: "-838:59:59"}, } for _, test := range tests { @@ -458,6 +467,34 @@ func TestMakeTimeBinaryLiteralBindAndExecute(t *testing.T) { } } +func TestMakeTimeExtremeExactSecondBindAndExecute(t *testing.T) { + tests := []struct { + name string + sql string + }{ + {"exponent", "select maketime(12, 34, '1e" + strings.Repeat("9", 8192) + "')"}, + {"mantissa", "select maketime(12, 34, '" + strings.Repeat("9", 4097) + "')"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mock := NewMockOptimizer(false) + pl, err := runOneExprStmt(mock, t, test.sql) + require.NoError(t, err) + + expr := pl.GetQuery().Nodes[1].ProjectList[0] + proc := testutil.NewProc(t) + defer proc.Free() + executor, err := colexec.NewExpressionExecutor(proc, expr) + require.NoError(t, err) + defer executor.Free() + result, err := executor.Eval(proc, nil, nil) + require.NoError(t, err) + require.True(t, result.GetNulls().Contains(0)) + }) + } +} + func makeTimeExpr(s string, p int32) *plan.Expr { dt, _ := types.ParseTime(s, 0) return &plan.Expr{ diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index 3fed76a61c63c..f6feee2977843 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7376,6 +7376,38 @@ func makeTimeFloatGetter[T constraints.Float](vec *vector.Vector) func(uint64) ( } } +func makeTimeDecimalIntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) { + param := vector.GenerateFunctionFixedTypeParameter[types.Decimal128](vec) + scale := vec.GetType().Scale + return func(i uint64) (int64, bool) { + value, null := param.GetValue(i) + if null { + return 0, true + } + exact, ok := new(big.Rat).SetString(value.Format(scale)) + if !ok { + return 0, true + } + negative := exact.Sign() < 0 + numerator := new(big.Int).Abs(exact.Num()) + rounded, remainder := new(big.Int), new(big.Int) + rounded.QuoRem(numerator, exact.Denom(), remainder) + if new(big.Int).Lsh(remainder, 1).Cmp(exact.Denom()) >= 0 { + rounded.Add(rounded, big.NewInt(1)) + } + if negative { + rounded.Neg(rounded) + } + if rounded.IsInt64() { + return rounded.Int64(), false + } + if negative { + return math.MinInt64, false + } + return math.MaxInt64, false + } +} + func makeTimeStringIntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) { param := vector.GenerateFunctionStrParameter(vec) isBinary := vec.GetIsBin() @@ -7489,6 +7521,8 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr if ivecs[0].GetType().Oid.IsMySQLString() { getHourValue = makeTimeStringIntegerGetter(ivecs[0]) + } else if ivecs[0].GetType().Oid == types.T_decimal128 { + getHourValue = makeTimeDecimalIntegerGetter(ivecs[0]) } else if getter, ok := makeTimeIntegerGetter(ivecs[0]); ok { getHourValue = getter } else { @@ -7519,6 +7553,8 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr if ivecs[1].GetType().Oid.IsMySQLString() { getMinuteValue = makeTimeStringIntegerGetter(ivecs[1]) + } else if ivecs[1].GetType().Oid == types.T_decimal128 { + getMinuteValue = makeTimeDecimalIntegerGetter(ivecs[1]) } else if getter, ok := makeTimeIntegerGetter(ivecs[1]); ok { getMinuteValue = getter } else { diff --git a/pkg/sql/plan/function/function_test.go b/pkg/sql/plan/function/function_test.go index d12670981c16a..c3c4c00142999 100644 --- a/pkg/sql/plan/function/function_test.go +++ b/pkg/sql/plan/function/function_test.go @@ -385,6 +385,36 @@ func TestMakeTimeReturnScale(t *testing.T) { require.Equal(t, types.T_time.ToTypeWithScale(6), defaultFloatResult.retType) } +func TestMakeTimeDecimalHourMinuteUseExactOverloads(t *testing.T) { + proc := testutil.NewProcess(t) + decimalType := types.New(types.T_decimal128, 30, 20) + + tests := []struct { + inputs []types.Type + args []types.T + }{ + {[]types.Type{decimalType, types.T_int64.ToType(), types.T_int64.ToType()}, []types.T{types.T_decimal128, types.T_float64, types.T_float64}}, + {[]types.Type{decimalType, types.T_varchar.ToType(), types.T_int64.ToType()}, []types.T{types.T_decimal128, types.T_varchar, types.T_float64}}, + {[]types.Type{decimalType, decimalType, types.T_int64.ToType()}, []types.T{types.T_decimal128, types.T_decimal128, types.T_float64}}, + {[]types.Type{decimalType, types.T_int64.ToType(), types.T_varchar.ToType()}, []types.T{types.T_decimal128, types.T_float64, types.T_varchar}}, + {[]types.Type{decimalType, types.T_varchar.ToType(), types.T_varchar.ToType()}, []types.T{types.T_decimal128, types.T_varchar, types.T_varchar}}, + {[]types.Type{types.T_int64.ToType(), decimalType, types.T_int64.ToType()}, []types.T{types.T_float64, types.T_decimal128, types.T_float64}}, + {[]types.Type{types.T_varchar.ToType(), decimalType, types.T_int64.ToType()}, []types.T{types.T_varchar, types.T_decimal128, types.T_float64}}, + {[]types.Type{types.T_int64.ToType(), decimalType, types.T_varchar.ToType()}, []types.T{types.T_float64, types.T_decimal128, types.T_varchar}}, + {[]types.Type{types.T_varchar.ToType(), decimalType, types.T_varchar.ToType()}, []types.T{types.T_varchar, types.T_decimal128, types.T_varchar}}, + {[]types.Type{decimalType, decimalType, types.New(types.T_decimal128, 20, 6)}, []types.T{types.T_decimal128, types.T_decimal128, types.T_varchar}}, + } + + for _, test := range tests { + result, err := GetFunctionByName(proc.Ctx, "maketime", test.inputs) + require.NoError(t, err) + require.True(t, result.needCast) + selected, err := GetFunctionById(proc.Ctx, result.GetEncodedOverloadID()) + require.NoError(t, err) + require.Equal(t, test.args, selected.args) + } +} + func TestMakeTimeStringSecondUsesExactOverload(t *testing.T) { proc := testutil.NewProcess(t) diff --git a/pkg/sql/plan/function/list_builtIn.go b/pkg/sql/plan/function/list_builtIn.go index 792338d54e954..c2785c8292206 100644 --- a/pkg/sql/plan/function/list_builtIn.go +++ b/pkg/sql/plan/function/list_builtIn.go @@ -10803,16 +10803,22 @@ func makeTimeCheck(overloads []overload, inputs []types.Type) checkResult { return newCheckResultWithFailure(failedFunctionParametersWrong) } exactSecond := isMakeTimeTextType(inputs[2].Oid) || inputs[2].Oid.IsDecimal() - if !isMakeTimeTextType(inputs[0].Oid) && !isMakeTimeTextType(inputs[1].Oid) && !exactSecond { + exactHour := inputs[0].Oid.IsDecimal() + exactMinute := inputs[1].Oid.IsDecimal() + if !isMakeTimeTextType(inputs[0].Oid) && !isMakeTimeTextType(inputs[1].Oid) && !exactHour && !exactMinute && !exactSecond { return fixedTypeMatch(overloads, inputs) } targetOids := []types.T{types.T_float64, types.T_float64, types.T_float64} if isMakeTimeTextType(inputs[0].Oid) { targetOids[0] = types.T_varchar + } else if exactHour { + targetOids[0] = types.T_decimal128 } if isMakeTimeTextType(inputs[1].Oid) { targetOids[1] = types.T_varchar + } else if exactMinute { + targetOids[1] = types.T_decimal128 } if exactSecond { targetOids[2] = types.T_varchar @@ -11398,6 +11404,66 @@ var supportedControlBuiltIns = []FuncNew{ return MakeTime }, }, + { + overloadId: 12, + args: []types.T{types.T_decimal128, types.T_float64, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 13, + args: []types.T{types.T_decimal128, types.T_varchar, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 14, + args: []types.T{types.T_decimal128, types.T_decimal128, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 15, + args: []types.T{types.T_decimal128, types.T_float64, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 16, + args: []types.T{types.T_decimal128, types.T_varchar, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 17, + args: []types.T{types.T_decimal128, types.T_decimal128, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 18, + args: []types.T{types.T_float64, types.T_decimal128, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 19, + args: []types.T{types.T_varchar, types.T_decimal128, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 20, + args: []types.T{types.T_float64, types.T_decimal128, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 21, + args: []types.T{types.T_varchar, types.T_decimal128, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, }, }, diff --git a/test/distributed/cases/function/func_datetime_maketime.result b/test/distributed/cases/function/func_datetime_maketime.result index 6c2958af750bd..7f8f25be009dd 100644 --- a/test/distributed/cases/function/func_datetime_maketime.result +++ b/test/distributed/cases/function/func_datetime_maketime.result @@ -100,6 +100,9 @@ varchar_at_half_microsecond SELECT CAST(MAKETIME(12, 59, CAST('59.99999949999999999' AS DECIMAL(30,20))) AS VARCHAR) AS decimal_below_half_microsecond; decimal_below_half_microsecond 12:59:59.999999 +SELECT CAST(MAKETIME(12, CAST('59.49999999999999999999' AS DECIMAL(30,20)), CAST('0' AS DECIMAL(2,1))) AS VARCHAR) AS decimal_minute_below_half; +decimal_minute_below_half +12:59:00.0 SELECT CAST(MAKETIME(X'0102', 0, 0) AS VARCHAR) AS binary_hour; binary_hour 258:00:00 diff --git a/test/distributed/cases/function/func_datetime_maketime.test b/test/distributed/cases/function/func_datetime_maketime.test index dbbbc080883ca..994f84556017f 100644 --- a/test/distributed/cases/function/func_datetime_maketime.test +++ b/test/distributed/cases/function/func_datetime_maketime.test @@ -49,6 +49,9 @@ SELECT CAST(MAKETIME(12, 59, '59.99999949999999999') AS VARCHAR) AS varchar_belo SELECT CAST(MAKETIME(12, 59, '59.9999995') AS VARCHAR) AS varchar_at_half_microsecond; SELECT CAST(MAKETIME(12, 59, CAST('59.99999949999999999' AS DECIMAL(30,20))) AS VARCHAR) AS decimal_below_half_microsecond; +# DECIMAL hour/minute round exactly without crossing a half boundary through FLOAT64 +SELECT CAST(MAKETIME(12, CAST('59.49999999999999999999' AS DECIMAL(30,20)), CAST('0' AS DECIMAL(2,1))) AS VARCHAR) AS decimal_minute_below_half; + # Binary literals retain numeric byte semantics instead of being parsed as text SELECT CAST(MAKETIME(X'0102', 0, 0) AS VARCHAR) AS binary_hour; SELECT CAST(MAKETIME(12, 0, X'01') AS VARCHAR) AS binary_second; From 3513b23431994225264db1ec1ec7c0d6983b679c Mon Sep 17 00:00:00 2001 From: VioletQwQ-0 <1659648118@qq.com> Date: Wed, 22 Jul 2026 17:58:27 +0800 Subject: [PATCH 29/30] fix datetime review edge cases --- pkg/sql/plan/build_expr_test.go | 9 ++ pkg/sql/plan/function/func_binary.go | 85 +++++++++++----- pkg/sql/plan/function/func_binary_test.go | 26 ++++- pkg/sql/plan/function/function_test.go | 41 ++++++++ pkg/sql/plan/function/list_builtIn.go | 96 ++++++++++++++++++- .../function/func_datetime_time_format.result | 4 +- 6 files changed, 230 insertions(+), 31 deletions(-) diff --git a/pkg/sql/plan/build_expr_test.go b/pkg/sql/plan/build_expr_test.go index 4a0cfd4086bf9..5582c4e262349 100644 --- a/pkg/sql/plan/build_expr_test.go +++ b/pkg/sql/plan/build_expr_test.go @@ -438,6 +438,15 @@ func TestMakeTimeBinaryLiteralBindAndExecute(t *testing.T) { {name: "negative decimal hour at half", sql: "select cast(maketime(cast('-12.5' as decimal(3, 1)), 0, 0) as varchar)", want: "-13:00:00"}, {name: "decimal hour positive overflow", sql: "select cast(maketime(cast('99999999999999999999999999999999999999' as decimal(38, 0)), 0, 0) as varchar)", want: "838:59:59"}, {name: "decimal hour negative overflow", sql: "select cast(maketime(cast('-99999999999999999999999999999999999999' as decimal(38, 0)), 0, 0) as varchar)", want: "-838:59:59"}, + {name: "decimal256 hour below half", sql: "select cast(maketime(cast('12.499999999999999999999999999999' as decimal(65, 30)), 0, 0) as varchar)", want: "12:00:00"}, + {name: "decimal256 hour at half", sql: "select cast(maketime(cast('12.500000000000000000000000000000' as decimal(65, 30)), 0, 0) as varchar)", want: "13:00:00"}, + {name: "decimal256 minute below half", sql: "select cast(maketime(12, cast('58.499999999999999999999999999999' as decimal(65, 30)), 0) as varchar)", want: "12:58:00"}, + {name: "decimal256 minute at half", sql: "select cast(maketime(12, cast('58.500000000000000000000000000000' as decimal(65, 30)), 0) as varchar)", want: "12:59:00"}, + {name: "decimal256 hour positive overflow", sql: "select cast(maketime(cast('99999999999999999999999999999999999999999999999999999999999999999' as decimal(65, 0)), 0, 0) as varchar)", want: "838:59:59"}, + {name: "decimal256 hour negative overflow", sql: "select cast(maketime(cast('-99999999999999999999999999999999999999999999999999999999999999999' as decimal(65, 0)), 0, 0) as varchar)", want: "-838:59:59"}, + {name: "safe exponent underflow", sql: "select cast(maketime(12, 34, '1e-5000') as varchar)", want: "12:34:00.000000"}, + {name: "zero mantissa huge exponent", sql: "select cast(maketime(12, 34, '0e5000') as varchar)", want: "12:34:00.000000"}, + {name: "wide zero mantissa", sql: "select cast(maketime(12, 34, '" + strings.Repeat("0", 4097) + "') as varchar)", want: "12:34:00.000000"}, } for _, test := range tests { diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index f6feee2977843..cad3bac1dec73 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -4019,11 +4019,12 @@ func TimeFormat(ivecs []*vector.Vector, result vector.FunctionResultWrapper, pro times := vector.GenerateFunctionFixedTypeParameter[types.Time](ivecs[0]) formats := vector.GenerateFunctionStrParameter(ivecs[1]) fmt, null2 := formats.GetStrValue(0) + emptyFormat := len(fmt) == 0 var buf bytes.Buffer for i := uint64(0); i < uint64(length); i++ { t, null1 := times.GetValue(i) - if null1 || null2 { + if null1 || null2 || emptyFormat { if err = rs.AppendBytes(nil, true); err != nil { return err } @@ -7376,7 +7377,31 @@ func makeTimeFloatGetter[T constraints.Float](vec *vector.Vector) func(uint64) ( } } -func makeTimeDecimalIntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) { +func makeTimeExactInteger(value string) (int64, bool) { + exact, ok := new(big.Rat).SetString(value) + if !ok { + return 0, true + } + negative := exact.Sign() < 0 + numerator := new(big.Int).Abs(exact.Num()) + rounded, remainder := new(big.Int), new(big.Int) + rounded.QuoRem(numerator, exact.Denom(), remainder) + if new(big.Int).Lsh(remainder, 1).Cmp(exact.Denom()) >= 0 { + rounded.Add(rounded, big.NewInt(1)) + } + if negative { + rounded.Neg(rounded) + } + if rounded.IsInt64() { + return rounded.Int64(), false + } + if negative { + return math.MinInt64, false + } + return math.MaxInt64, false +} + +func makeTimeDecimal128IntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) { param := vector.GenerateFunctionFixedTypeParameter[types.Decimal128](vec) scale := vec.GetType().Scale return func(i uint64) (int64, bool) { @@ -7384,27 +7409,19 @@ func makeTimeDecimalIntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) if null { return 0, true } - exact, ok := new(big.Rat).SetString(value.Format(scale)) - if !ok { + return makeTimeExactInteger(value.Format(scale)) + } +} + +func makeTimeDecimal256IntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) { + param := vector.GenerateFunctionFixedTypeParameter[types.Decimal256](vec) + scale := vec.GetType().Scale + return func(i uint64) (int64, bool) { + value, null := param.GetValue(i) + if null { return 0, true } - negative := exact.Sign() < 0 - numerator := new(big.Int).Abs(exact.Num()) - rounded, remainder := new(big.Int), new(big.Int) - rounded.QuoRem(numerator, exact.Denom(), remainder) - if new(big.Int).Lsh(remainder, 1).Cmp(exact.Denom()) >= 0 { - rounded.Add(rounded, big.NewInt(1)) - } - if negative { - rounded.Neg(rounded) - } - if rounded.IsInt64() { - return rounded.Int64(), false - } - if negative { - return math.MinInt64, false - } - return math.MaxInt64, false + return makeTimeExactInteger(value.Format(scale)) } } @@ -7426,6 +7443,7 @@ func makeTimeStringIntegerGetter(vec *vector.Vector) func(uint64) (int64, bool) func makeTimeExactSecond(value string) (int64, uint32, bool) { const maxExactSecondDigits = 4096 + const maxExactSecondExponent = maxExactSecondDigits + 7 value = strings.TrimSpace(value) if len(value) == 0 { @@ -7437,13 +7455,16 @@ func makeTimeExactSecond(value string) (int64, uint32, bool) { end++ } digits := 0 + nonzeroMantissa := false for end < len(value) && value[end] >= '0' && value[end] <= '9' { + nonzeroMantissa = nonzeroMantissa || value[end] != '0' end++ digits++ } if end < len(value) && value[end] == '.' { end++ for end < len(value) && value[end] >= '0' && value[end] <= '9' { + nonzeroMantissa = nonzeroMantissa || value[end] != '0' end++ digits++ } @@ -7451,24 +7472,34 @@ func makeTimeExactSecond(value string) (int64, uint32, bool) { if digits == 0 { return 0, 0, false } + if !nonzeroMantissa { + return 0, 0, false + } + if digits > maxExactSecondDigits { + return 0, 0, true + } if end < len(value) && (value[end] == 'e' || value[end] == 'E') { exponentStart := end end++ + negativeExponent := false if end < len(value) && (value[end] == '+' || value[end] == '-') { + negativeExponent = value[end] == '-' end++ } exponentDigits := end exponentMagnitude := 0 for end < len(value) && value[end] >= '0' && value[end] <= '9' { - if exponentMagnitude <= maxExactSecondDigits { + if exponentMagnitude <= maxExactSecondExponent { exponentMagnitude = exponentMagnitude*10 + int(value[end]-'0') } end++ } if end == exponentDigits { end = exponentStart - } else if exponentMagnitude > maxExactSecondDigits { + } else if negativeExponent && exponentMagnitude > maxExactSecondExponent { + return 0, 0, false + } else if exponentMagnitude > maxExactSecondExponent { return 0, 0, true } } @@ -7522,7 +7553,9 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr if ivecs[0].GetType().Oid.IsMySQLString() { getHourValue = makeTimeStringIntegerGetter(ivecs[0]) } else if ivecs[0].GetType().Oid == types.T_decimal128 { - getHourValue = makeTimeDecimalIntegerGetter(ivecs[0]) + getHourValue = makeTimeDecimal128IntegerGetter(ivecs[0]) + } else if ivecs[0].GetType().Oid == types.T_decimal256 { + getHourValue = makeTimeDecimal256IntegerGetter(ivecs[0]) } else if getter, ok := makeTimeIntegerGetter(ivecs[0]); ok { getHourValue = getter } else { @@ -7554,7 +7587,9 @@ func MakeTime(ivecs []*vector.Vector, result vector.FunctionResultWrapper, _ *pr if ivecs[1].GetType().Oid.IsMySQLString() { getMinuteValue = makeTimeStringIntegerGetter(ivecs[1]) } else if ivecs[1].GetType().Oid == types.T_decimal128 { - getMinuteValue = makeTimeDecimalIntegerGetter(ivecs[1]) + getMinuteValue = makeTimeDecimal128IntegerGetter(ivecs[1]) + } else if ivecs[1].GetType().Oid == types.T_decimal256 { + getMinuteValue = makeTimeDecimal256IntegerGetter(ivecs[1]) } else if getter, ok := makeTimeIntegerGetter(ivecs[1]); ok { getMinuteValue = getter } else { diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index 75356cce4f250..bec6e50815084 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9440,7 +9440,7 @@ func initTimeFormatTestCase() []tcTemp { []bool{false}), }, { - info: "test time_format - empty format returns empty string", + info: "test time_format - empty format returns null", inputs: []FunctionTestInput{ NewFunctionTestInput(types.T_time.ToType(), []types.Time{t1, t6}, @@ -9449,7 +9449,7 @@ func initTimeFormatTestCase() []tcTemp { }, expect: NewFunctionTestResult(types.T_varchar.ToType(), false, []string{"", ""}, - []bool{false, false}), + []bool{true, true}), }, { info: "test time_format - %h:%i:%s %p", @@ -9743,6 +9743,28 @@ func TestMakeTimeExactStringSecondRounding(t *testing.T) { _, _, null := makeTimeExactSecond("1e999999999") require.True(t, null, "MAKETIME must reject an unbounded exponent without allocating it") + + second, microsecond, null := makeTimeExactSecond("1e-5000") + require.False(t, null) + require.Zero(t, second) + require.Zero(t, microsecond) + + second, microsecond, null = makeTimeExactSecond("0e5000") + require.False(t, null) + require.Zero(t, second) + require.Zero(t, microsecond) + + second, microsecond, null = makeTimeExactSecond(strings.Repeat("0", 4097)) + require.False(t, null) + require.Zero(t, second) + require.Zero(t, microsecond) + + for _, value := range []string{"1e-4103", "1e-4104"} { + second, microsecond, null = makeTimeExactSecond(value) + require.False(t, null, value) + require.Zero(t, second, value) + require.Zero(t, microsecond, value) + } } func TestMakeTimeStringHourMinuteSemantics(t *testing.T) { diff --git a/pkg/sql/plan/function/function_test.go b/pkg/sql/plan/function/function_test.go index c3c4c00142999..e2e77a489b845 100644 --- a/pkg/sql/plan/function/function_test.go +++ b/pkg/sql/plan/function/function_test.go @@ -388,6 +388,7 @@ func TestMakeTimeReturnScale(t *testing.T) { func TestMakeTimeDecimalHourMinuteUseExactOverloads(t *testing.T) { proc := testutil.NewProcess(t) decimalType := types.New(types.T_decimal128, 30, 20) + decimal256Type := types.New(types.T_decimal256, 65, 30) tests := []struct { inputs []types.Type @@ -403,6 +404,11 @@ func TestMakeTimeDecimalHourMinuteUseExactOverloads(t *testing.T) { {[]types.Type{types.T_int64.ToType(), decimalType, types.T_varchar.ToType()}, []types.T{types.T_float64, types.T_decimal128, types.T_varchar}}, {[]types.Type{types.T_varchar.ToType(), decimalType, types.T_varchar.ToType()}, []types.T{types.T_varchar, types.T_decimal128, types.T_varchar}}, {[]types.Type{decimalType, decimalType, types.New(types.T_decimal128, 20, 6)}, []types.T{types.T_decimal128, types.T_decimal128, types.T_varchar}}, + {[]types.Type{decimal256Type, types.T_int64.ToType(), types.T_int64.ToType()}, []types.T{types.T_decimal256, types.T_float64, types.T_float64}}, + {[]types.Type{types.T_int64.ToType(), decimal256Type, types.T_int64.ToType()}, []types.T{types.T_float64, types.T_decimal256, types.T_float64}}, + {[]types.Type{decimal256Type, decimalType, types.T_varchar.ToType()}, []types.T{types.T_decimal256, types.T_decimal128, types.T_varchar}}, + {[]types.Type{decimalType, decimal256Type, types.T_varchar.ToType()}, []types.T{types.T_decimal128, types.T_decimal256, types.T_varchar}}, + {[]types.Type{decimal256Type, decimal256Type, types.T_varchar.ToType()}, []types.T{types.T_decimal256, types.T_decimal256, types.T_varchar}}, } for _, test := range tests { @@ -415,6 +421,41 @@ func TestMakeTimeDecimalHourMinuteUseExactOverloads(t *testing.T) { } } +func TestMakeTimeDecimal256OverloadMatrix(t *testing.T) { + proc := testutil.NewProcess(t) + decimal128Type := types.New(types.T_decimal128, 30, 20) + decimal256Type := types.New(types.T_decimal256, 65, 30) + type typeChoice struct { + input types.Type + target types.T + } + hourMinuteChoices := []typeChoice{ + {types.T_int64.ToType(), types.T_float64}, + {types.T_varchar.ToType(), types.T_varchar}, + {decimal128Type, types.T_decimal128}, + {decimal256Type, types.T_decimal256}, + } + secondChoices := []typeChoice{ + {types.T_int64.ToType(), types.T_float64}, + {types.T_varchar.ToType(), types.T_varchar}, + } + + for _, hour := range hourMinuteChoices { + for _, minute := range hourMinuteChoices { + if hour.target != types.T_decimal256 && minute.target != types.T_decimal256 { + continue + } + for _, second := range secondChoices { + result, err := GetFunctionByName(proc.Ctx, "maketime", []types.Type{hour.input, minute.input, second.input}) + require.NoError(t, err) + selected, err := GetFunctionById(proc.Ctx, result.GetEncodedOverloadID()) + require.NoError(t, err) + require.Equal(t, []types.T{hour.target, minute.target, second.target}, selected.args) + } + } + } +} + func TestMakeTimeStringSecondUsesExactOverload(t *testing.T) { proc := testutil.NewProcess(t) diff --git a/pkg/sql/plan/function/list_builtIn.go b/pkg/sql/plan/function/list_builtIn.go index c2785c8292206..bd99b14a70a2b 100644 --- a/pkg/sql/plan/function/list_builtIn.go +++ b/pkg/sql/plan/function/list_builtIn.go @@ -10813,12 +10813,20 @@ func makeTimeCheck(overloads []overload, inputs []types.Type) checkResult { if isMakeTimeTextType(inputs[0].Oid) { targetOids[0] = types.T_varchar } else if exactHour { - targetOids[0] = types.T_decimal128 + if inputs[0].Oid == types.T_decimal256 { + targetOids[0] = types.T_decimal256 + } else { + targetOids[0] = types.T_decimal128 + } } if isMakeTimeTextType(inputs[1].Oid) { targetOids[1] = types.T_varchar } else if exactMinute { - targetOids[1] = types.T_decimal128 + if inputs[1].Oid == types.T_decimal256 { + targetOids[1] = types.T_decimal256 + } else { + targetOids[1] = types.T_decimal128 + } } if exactSecond { targetOids[2] = types.T_varchar @@ -11464,6 +11472,90 @@ var supportedControlBuiltIns = []FuncNew{ retType: makeTimeReturnType, newOp: func() executeLogicOfOverload { return MakeTime }, }, + { + overloadId: 22, + args: []types.T{types.T_decimal256, types.T_float64, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 23, + args: []types.T{types.T_decimal256, types.T_varchar, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 24, + args: []types.T{types.T_decimal256, types.T_decimal128, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 25, + args: []types.T{types.T_decimal256, types.T_decimal256, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 26, + args: []types.T{types.T_decimal256, types.T_float64, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 27, + args: []types.T{types.T_decimal256, types.T_varchar, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 28, + args: []types.T{types.T_decimal256, types.T_decimal128, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 29, + args: []types.T{types.T_decimal256, types.T_decimal256, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 30, + args: []types.T{types.T_float64, types.T_decimal256, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 31, + args: []types.T{types.T_varchar, types.T_decimal256, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 32, + args: []types.T{types.T_decimal128, types.T_decimal256, types.T_float64}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 33, + args: []types.T{types.T_float64, types.T_decimal256, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 34, + args: []types.T{types.T_varchar, types.T_decimal256, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, + { + overloadId: 35, + args: []types.T{types.T_decimal128, types.T_decimal256, types.T_varchar}, + retType: makeTimeReturnType, + newOp: func() executeLogicOfOverload { return MakeTime }, + }, }, }, diff --git a/test/distributed/cases/function/func_datetime_time_format.result b/test/distributed/cases/function/func_datetime_time_format.result index 49c35ca4829e2..9d085e457fff0 100644 --- a/test/distributed/cases/function/func_datetime_time_format.result +++ b/test/distributed/cases/function/func_datetime_time_format.result @@ -78,10 +78,10 @@ null_format null SELECT TIME_FORMAT('15:30:45', '') AS empty_positive_format; empty_positive_format - +null SELECT TIME_FORMAT('-15:30:45', '') AS empty_negative_format; empty_negative_format - +null CREATE TABLE t1(t TIME); INSERT INTO t1 VALUES ('15:30:45'), ('00:00:00'), ('23:59:59'), ('12:34:56'); SELECT t, TIME_FORMAT(t, '%H:%i:%s') AS formatted FROM t1; From 3998fb2679d808a1a136abce3b2a1cc682141466 Mon Sep 17 00:00:00 2001 From: VioletQwQ-0 <1659648118@qq.com> Date: Thu, 23 Jul 2026 11:04:38 +0800 Subject: [PATCH 30/30] fix(function): normalize exact maketime seconds --- pkg/sql/plan/build_expr_test.go | 3 + pkg/sql/plan/function/func_binary.go | 94 +++++++++++++++++++---- pkg/sql/plan/function/func_binary_test.go | 15 ++++ 3 files changed, 96 insertions(+), 16 deletions(-) diff --git a/pkg/sql/plan/build_expr_test.go b/pkg/sql/plan/build_expr_test.go index 5582c4e262349..e32976bb7a8ae 100644 --- a/pkg/sql/plan/build_expr_test.go +++ b/pkg/sql/plan/build_expr_test.go @@ -447,6 +447,9 @@ func TestMakeTimeBinaryLiteralBindAndExecute(t *testing.T) { {name: "safe exponent underflow", sql: "select cast(maketime(12, 34, '1e-5000') as varchar)", want: "12:34:00.000000"}, {name: "zero mantissa huge exponent", sql: "select cast(maketime(12, 34, '0e5000') as varchar)", want: "12:34:00.000000"}, {name: "wide zero mantissa", sql: "select cast(maketime(12, 34, '" + strings.Repeat("0", 4097) + "') as varchar)", want: "12:34:00.000000"}, + {name: "wide leading zero second", sql: "select cast(maketime(12, 34, '" + strings.Repeat("0", 4096) + "1') as varchar)", want: "12:34:01.000000"}, + {name: "wide trailing fractional zero second", sql: "select cast(maketime(12, 34, '1." + strings.Repeat("0", 4097) + "') as varchar)", want: "12:34:01.000000"}, + {name: "wide zero padding canceled by exponent", sql: "select cast(maketime(12, 34, '0." + strings.Repeat("0", 4096) + "1e4097') as varchar)", want: "12:34:01.000000"}, } for _, test := range tests { diff --git a/pkg/sql/plan/function/func_binary.go b/pkg/sql/plan/function/func_binary.go index cad3bac1dec73..d1a2c21062c78 100644 --- a/pkg/sql/plan/function/func_binary.go +++ b/pkg/sql/plan/function/func_binary.go @@ -7451,34 +7451,56 @@ func makeTimeExactSecond(value string) (int64, uint32, bool) { } end := 0 + negative := false if value[end] == '+' || value[end] == '-' { + negative = value[end] == '-' end++ } - digits := 0 - nonzeroMantissa := false + + totalDigits := 0 + firstNonzeroDigit := -1 + lastNonzeroDigit := -1 + integerStart := end for end < len(value) && value[end] >= '0' && value[end] <= '9' { - nonzeroMantissa = nonzeroMantissa || value[end] != '0' + if value[end] != '0' { + if firstNonzeroDigit == -1 { + firstNonzeroDigit = totalDigits + } + lastNonzeroDigit = totalDigits + } end++ - digits++ + totalDigits++ } + integerEnd := end + fractionStart := end + fractionEnd := end if end < len(value) && value[end] == '.' { end++ + fractionStart = end for end < len(value) && value[end] >= '0' && value[end] <= '9' { - nonzeroMantissa = nonzeroMantissa || value[end] != '0' + if value[end] != '0' { + if firstNonzeroDigit == -1 { + firstNonzeroDigit = totalDigits + } + lastNonzeroDigit = totalDigits + } end++ - digits++ + totalDigits++ } + fractionEnd = end } - if digits == 0 { + if totalDigits == 0 { return 0, 0, false } - if !nonzeroMantissa { + if firstNonzeroDigit == -1 { return 0, 0, false } - if digits > maxExactSecondDigits { + significantDigits := lastNonzeroDigit - firstNonzeroDigit + 1 + if significantDigits > maxExactSecondDigits { return 0, 0, true } + exponent := 0 if end < len(value) && (value[end] == 'e' || value[end] == 'E') { exponentStart := end end++ @@ -7489,25 +7511,65 @@ func makeTimeExactSecond(value string) (int64, uint32, bool) { } exponentDigits := end exponentMagnitude := 0 + exponentLimit := maxExactSecondExponent + totalDigits + exponentOverflow := false for end < len(value) && value[end] >= '0' && value[end] <= '9' { - if exponentMagnitude <= maxExactSecondExponent { - exponentMagnitude = exponentMagnitude*10 + int(value[end]-'0') + digit := int(value[end] - '0') + if !exponentOverflow { + if exponentMagnitude > (exponentLimit-digit)/10 { + exponentOverflow = true + } else { + exponentMagnitude = exponentMagnitude*10 + digit + } } end++ } if end == exponentDigits { end = exponentStart - } else if negativeExponent && exponentMagnitude > maxExactSecondExponent { - return 0, 0, false - } else if exponentMagnitude > maxExactSecondExponent { + } else if exponentOverflow { + if negativeExponent { + return 0, 0, false + } return 0, 0, true + } else if negativeExponent { + exponent = -exponentMagnitude + } else { + exponent = exponentMagnitude } } - if end > maxExactSecondDigits { + + fractionDigits := fractionEnd - fractionStart + trailingZeroDigits := totalDigits - lastNonzeroDigit - 1 + exponent += trailingZeroDigits - fractionDigits + if exponent < -maxExactSecondExponent { + return 0, 0, false + } + if exponent > maxExactSecondExponent { return 0, 0, true } - second, ok := new(big.Rat).SetString(value[:end]) + var normalized strings.Builder + normalized.Grow(significantDigits + 16) + if negative { + normalized.WriteByte('-') + } + digitIndex := 0 + appendSignificantDigits := func(part string) { + for i := range part { + if digitIndex >= firstNonzeroDigit && digitIndex <= lastNonzeroDigit { + normalized.WriteByte(part[i]) + } + digitIndex++ + } + } + appendSignificantDigits(value[integerStart:integerEnd]) + appendSignificantDigits(value[fractionStart:fractionEnd]) + if exponent != 0 { + normalized.WriteByte('e') + normalized.WriteString(strconv.Itoa(exponent)) + } + + second, ok := new(big.Rat).SetString(normalized.String()) if !ok || second.Sign() < 0 || second.Cmp(big.NewRat(60, 1)) >= 0 { return 0, 0, true } diff --git a/pkg/sql/plan/function/func_binary_test.go b/pkg/sql/plan/function/func_binary_test.go index bec6e50815084..0471c9beaf21f 100644 --- a/pkg/sql/plan/function/func_binary_test.go +++ b/pkg/sql/plan/function/func_binary_test.go @@ -9759,6 +9759,21 @@ func TestMakeTimeExactStringSecondRounding(t *testing.T) { require.Zero(t, second) require.Zero(t, microsecond) + for _, test := range []struct { + name string + value string + }{ + {name: "wide leading zeroes", value: strings.Repeat("0", 4096) + "1"}, + {name: "wide trailing fractional zeroes", value: "1." + strings.Repeat("0", 4097)}, + {name: "wide fractional leading zeroes canceled by exponent", value: "0." + strings.Repeat("0", 4096) + "1e4097"}, + {name: "wide integer trailing zeroes canceled by exponent", value: "1" + strings.Repeat("0", 4096) + "e-4096"}, + } { + second, microsecond, null = makeTimeExactSecond(test.value) + require.False(t, null, test.name) + require.Equal(t, int64(1), second, test.name) + require.Zero(t, microsecond, test.name) + } + for _, value := range []string{"1e-4103", "1e-4104"} { second, microsecond, null = makeTimeExactSecond(value) require.False(t, null, value)