Skip to content

Refactor and improve DATE parsing and SQL_MODE handling - #3723

Merged
jycor merged 69 commits into
mainfrom
james/date
Sep 8, 2026
Merged

Refactor and improve DATE parsing and SQL_MODE handling#3723
jycor merged 69 commits into
mainfrom
james/date

Conversation

@jycor

@jycor jycor commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

This PR improves support for parsing various strings into DATETIME.
There are a few improvements to converting DATETIME types to other types, but just enough to get exist tests to pass. Improvements to that area will come in future PRs.
Warnings have also been improved to match MySQL, but we are currently ignoring the delimiter ones.

Dolt bump: dolthub/dolt#11584

Changes:

  • Default SQL_MODE matches MySQL.
  • The time.Time package does not fully support ZERO_IN_DATE, so that SQL_MODE isn't supported for now.
  • Refactor datetimeType.Convert()
  • Tests

Fixes:

Possibly a breaking change, because of SQL_MODE changes.

@jycor
jycor force-pushed the james/date branch 4 times, most recently from 8f3e5ab to 2392f69 Compare August 28, 2026 21:32
@jycor jycor changed the title Date Conversions Refactor and improve DATE parsing and SQL_MODE handling Sep 1, 2026
Comment thread enginetest/queries/queries.go
Comment thread enginetest/queries/script_queries.go
Comment thread sql/expression/function/json/json_object.go
@itoqa

itoqa Bot commented Sep 4, 2026

Copy link
Copy Markdown

Ito QA test results
Commit: 7429324: 19 test cases ran, 2 failed ❌, 17 passed ✅.

Summary

Coverage spans temporal parsing, arithmetic, date functions, time-zone conversion, NULL handling, boundary and malformed inputs, persistence, and JSON serialization. It exercises both normal product behavior and adversarial edge cases, with most established behavior remaining healthy but gaps at unusual date and malformed-key boundaries.

Merge with caution — this PR introduces medium-severity data-integrity risks in uncommon temporal and JSON edge cases, including silently altered dates and corrupted objects instead of clean failures. The issues are directly attributable to the change and should be addressed before treating the merge as fully safe.

Tests run by Ito

View full run

Result Severity Type Description
Medium severity General A zero date with fractional seconds was changed into a different date instead of being kept as a zero date or rejected.
Medium severity General The query succeeds but returns {"": "late-value", "valid": "pair"} for an invalid datetime key. The wrapped form preserves the same corrupted object instead of reporting the invalid key.
Arithmetic Adding, subtracting, multiplying, dividing, taking the remainder, and using a bit operation on a datetime returned the expected numeric values with no warnings.
Arithmetic Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The observed result follows the repository's explicit temporal-numeric coercion design rather than violating an established invalid-input contract: a complete zero date is supported, deliberately formats as all zeroes, and DATETIME(0) is deliberately treated as an integer in arithmetic, so adding one yields one. The finding incorrectly imports the NULL/warning semantics of date-validating function…
General Malformed, unknown, and numeric timezone inputs returned NULL. SYSTEM and valid offset conversions still returned 14:00:00 with no warnings, so neighboring conversions continued normally.
General Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The finding assumes that a zero year is the same invalid partial-zero condition as a zero month or day, but the repository expressly defines otherwise and has extensive tests for the exact reported outputs. Acceptance of 0000-01-01 and extraction of 0/1/1 are deliberate compatibility behavior, not an accidental inconsistency; the PR-causation axis is nevertheless supported because the PR's parser …
Datetime Supported datetime values convert correctly. Fractional seconds are rounded to milliseconds, compact dates are accepted, and two-digit years map to 2069 and 1970 as expected.
Datetime Invalid dates and clock values return NULL with warning 1292 instead of becoming valid datetimes. The local SQL checks passed as expected.
Datetime Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The finding incorrectly treats a zero year as the unsupported ZERO_IN_DATE case. The parser's stated policy, direct conversion tests, SQL function expectations, and MySQL-oriented comments all affirm that 0000-01-01 is intentionally accepted as a non-zero time whose components are year 0, month 1, and day 1; therefore the observed 0/1/1 results are by design. The row's PR-causation claim is unsupp…
Division Division by zero and MOD by zero return NULL with warning 1365. Decimal division returns the expected value 0.50000.
Function Valid dates returned the expected year, month, quarter, day, weekday, day count, date difference, and month-end values.
Function Date functions returned zero components for the complete zero date and returned NULL with warning 1292 for the invalid date.
Function Date calculations returned the expected values for valid dates, empty values, zero dates, and the maximum supported date.
Json The JSON object is valid, the datetime key uses the expected SQL format, and both key/value pairs stay together.
Nulls NULL arithmetic and modulo return NULL, and comparisons with NULL return NULL. IS NULL still distinguishes NULL from a non-NULL value.
Timezone A time at UTC shifted from 12:00 to 14:00 at UTC+2. The result stayed a datetime and produced no warnings.
Timezone Invalid dates, numeric timezone values, and unknown timezone names all returned NULL safely. Valid conversion still worked in the same local SQL server.
Writes A partially valid date-time value was saved after its trailing text was cut off. The database returned warning 1292, and reading the row returned 2020-01-01 01:02:00.
Writes Inserting a date with month 13 was rejected, and the table stayed empty.

Tip

Reply with @itoqa to send us feedback on this test run.

Comment thread sql/types/datetime.go
Comment thread sql/expression/function/json/json_object.go
@itoqa

itoqa Bot commented Sep 4, 2026

Copy link
Copy Markdown

Ito QA test results

History reset (rebase or force-push detected). Starting test narrative over.

Commit: e02c016: 19 test cases ran, 2 failed ❌, 17 passed ✅.

Summary

The run covers core date/time and arithmetic behavior across normal operations, invalid inputs, boundary values, warnings, SQL-mode enforcement, and state after errors, with additional checks of assertion behavior. Most application behavior appears healthy, but coverage exposed a data-integrity gap in strict date handling and a validation-path inconsistency.

Not safe to merge yet — a high-severity PR-attributable defect allows invalid zero dates to be stored even when strict mode is enabled, risking corrupted user data; a separate medium-severity harness defect also leaves prepared assertions unreliable. These are merge-blocking findings rather than flag-for-later observations.

Tests run by Ito

View full run

Result Severity Type Description
High severity Mode A zero-date INSERT was accepted under the default strict mode, and a zero-date UPDATE also remained accepted. The expected behavior was to reject the write or emit the SQL-mode warning instead of storing a zero temporal value.
Medium severity Harness A prepared script assertion with warning checks disabled still runs the warning validation. The normal script and prepared query paths skip warning validation correctly, but the prepared script path does not.
Arithmetic Temporal addition, decimal arithmetic, integer division, modulo, bitwise arithmetic, and rounded decimal division all returned the expected values.
Arithmetic NULL operands returned NULL, and division or modulo by zero returned NULL with the expected Division by 0 warning. The native SQL checks passed; the browser error only reflects that the target speaks MySQL protocol instead of HTTP.
General MakeDate mapped years 0, 69, 70, and 99 to 2000, 2069, 1970, and 1999. Negative and overlarge years returned NULL instead of producing an invalid date.
General Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The reported outputs are statically explained by the implementation, but the claim that they are invalid contradicts the repository's explicit unit-test contract: non-leap-year day 366 is intentionally expected to roll into the next year. This is therefore intended behavior rather than a defect. Independently, the diff preserves the exact global-bound-plus-AddDate behavior from the base version, s…
General Invalid date casts return NULL with warning 1292 instead of failing the statement. Date comparisons and YEAR also keep that behavior, and a later valid query still returns 1.
General Valid dates return the correct positive and negative differences. Invalid dates return NULL with matching warnings in either order, and later valid queries still work.
General Invalid date text returns NULL with warning 1292 when used directly, inside a date function, and in a comparison. The next valid query still returns 1.
General Date values stay on the same day, while datetime and timestamp values round .9995 up to the next day. Values below the carry point keep their original day and fractional precision.
Datetime String, byte-compatible, and time values converted to the expected UTC results. DATE values dropped the time portion, while DATETIME values kept the requested fractional precision.
Datetime Malformed, out-of-range, and unsupported date and time values were rejected without returning a valid value. The conversion tests passed for DATE, DATETIME, and TIMESTAMP boundaries.
Function YEAR, QUARTER, and MONTH returned 2020, 1, and 1 for 2020-01-07. DATEDIFF returned 7 for the dates 2020-01-07 and 2019-12-31.
Function MakeDate with year 12 returned 2012-01-01, and year 70 returned 1970-01-01. Both results match the expected two-digit year rules.
Harness Script checks passed. Expected errors are still required even when warning checks are skipped.
Mode A fresh SQL session returned the six advertised default mode flags, and valid DATE, DATETIME, and TIMESTAMP values were stored and read back correctly.
Rev Strict SQL mode rejected a datetime value with invalid trailing text, and the table stayed empty.
Rev The database handled full zero dates, year-zero dates, and partial-zero dates according to the configured rules. Valid values were stored and read back, while unsupported partial-zero dates and year-zero timestamps were rejected.
Warning Malformed date and time casts return NULL and record the expected warning. The initial browser check used an HTTP request against a SQL endpoint, but the focused local engine tests passed after using the correct in-process path.

Tip

Reply with @itoqa to send us feedback on this test run.

Comment thread sql/types/datetime.go
@itoqa

itoqa Bot commented Sep 6, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Reporte02c0164339875: 14 test cases ran, 1 new failure ❌, 2 fixed ✅, 10 passing ✅, 1 additional finding ⚠️.

Diff Summary

The run broadly exercises SQL date/time behavior across valid reads and writes, arithmetic, boundary values, malformed inputs, warning handling, retries, and assertion/harness flows. Coverage includes both normal application behavior and adversarial edge cases, with healthy results for valid temporal operations but a problem in invalid-input handling.

Merge with caution — the PR introduces a medium-severity regression where malformed temporal input fails queries instead of returning the expected recoverable NULL and warning, which can disrupt applications that rely on that behavior. A separate similar finding is not attributable to this PR and is a flag for later rather than a merge driver.

Tests run by Ito

View full run

Result State Severity Type Description
❌ New Failure Medium severity Warnings A query that extracts the hour from invalid date text shows an error instead of returning NULL and a warning.
❌->✅ Fixed Harness The normal and prepared query checks both completed successfully. Malformed time input followed the expected error path without hiding or leaving behind a query result.
❌->✅ Fixed Mode Valid date and date-time changes were saved with the expected normalized values. An invalid change was rejected, and the existing row stayed unchanged.
Passing Arithmetic Adding two hours to 13:00 produces 15:00, and the numeric expressions return 42, 2, and 1 as expected.
Passing Assertions The table setup ran before the four checks, and the corrected checks completed successfully. The earlier error came from placeholder expressions in a disabled debug test, not from the product.
Passing General NULL input stayed NULL, malformed text and binary input returned a repeatable error, and the same malformed query produced the same diagnostic when repeated.
Passing General HOUR returned the expected values at one-hour boundaries, for negative times, for a datetime, and for NULL.
Passing General Explicit and default temporal values were stored in the expected format. Valid updates worked, invalid inserts and updates were rejected without changing existing rows, and automatic timestamp updates worked as expected.
Passing General The first script correctly rejected malformed time text. After changing it to a valid TIME value, the retry returned hour 13 with no warnings, matching a fresh successful run.
Passing Datetime Verified acceptable by independent adversarial review: the reported expectation does not match what the code actually promises. Review notes: The finding conflates support for the TIMESTAMP data type and TIMESTAMP(value) function with TIMESTAMP being a legal MySQL CAST target. Repository code and compatibility tests consistently expose timestamp conversion through TIMESTAMP(...) while the CAST target vocabulary omits TIMESTAMP, so a parser syntax error for CAST(... AS TIMESTAMP) is expected rather than a defect. The finding's separate p…
Passing Hour HOUR returns 20 for the datetime value, 13 for the time literal, and 13 for the native TIME value.
Passing Mode The session reported the expected default SQL mode, and valid date and time values were stored and read back correctly. The temporal default also filled the omitted column as expected.
Passing Rev HOUR returned 0, 13, 25, 2, and NULL for midnight, a normal time, an extended-hour time, a negative time, and a NULL value.
⏸️ Skipped Arithmetic Temporal addition, decimal arithmetic, integer division, modulo, bitwise arithmetic, and rounded decimal division all returned the expected values.
⏸️ Skipped Arithmetic NULL operands returned NULL, and division or modulo by zero returned NULL with the expected Division by 0 warning. The native SQL checks passed; the browser error only reflects that the target speaks MySQL protocol instead of HTTP.
⏸️ Skipped General MakeDate mapped years 0, 69, 70, and 99 to 2000, 2069, 1970, and 1999. Negative and overlarge years returned NULL instead of producing an invalid date.
⏸️ Skipped General Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The reported outputs are statically explained by the implementation, but the claim that they are invalid contradicts the repository's explicit unit-test contract: non-leap-year day 366 is intentionally expected to roll into the next year. This is therefore intended behavior rather than a defect. Independently, the diff preserves the exact global-bound-plus-AddDate behavior from the base version, s…
⏸️ Skipped General Invalid date casts return NULL with warning 1292 instead of failing the statement. Date comparisons and YEAR also keep that behavior, and a later valid query still returns 1.
⏸️ Skipped General Valid dates return the correct positive and negative differences. Invalid dates return NULL with matching warnings in either order, and later valid queries still work.
⏸️ Skipped General Invalid date text returns NULL with warning 1292 when used directly, inside a date function, and in a comparison. The next valid query still returns 1.
⏸️ Skipped General Date values stay on the same day, while datetime and timestamp values round .9995 up to the next day. Values below the carry point keep their original day and fractional precision.
⏸️ Skipped Datetime String, byte-compatible, and time values converted to the expected UTC results. DATE values dropped the time portion, while DATETIME values kept the requested fractional precision.
⏸️ Skipped Datetime Malformed, out-of-range, and unsupported date and time values were rejected without returning a valid value. The conversion tests passed for DATE, DATETIME, and TIMESTAMP boundaries.
⏸️ Skipped Function YEAR, QUARTER, and MONTH returned 2020, 1, and 1 for 2020-01-07. DATEDIFF returned 7 for the dates 2020-01-07 and 2019-12-31.
⏸️ Skipped Function MakeDate with year 12 returned 2012-01-01, and year 70 returned 1970-01-01. Both results match the expected two-digit year rules.
⏸️ Skipped Harness Script checks passed. Expected errors are still required even when warning checks are skipped.
⏸️ Skipped Mode A fresh SQL session returned the six advertised default mode flags, and valid DATE, DATETIME, and TIMESTAMP values were stored and read back correctly.
⏸️ Skipped Rev Strict SQL mode rejected a datetime value with invalid trailing text, and the table stayed empty.
⏸️ Skipped Rev The database handled full zero dates, year-zero dates, and partial-zero dates according to the configured rules. Valid values were stored and read back, while unsupported partial-zero dates and year-zero timestamps were rejected.
⏸️ Skipped Warning Malformed date and time casts return NULL and record the expected warning. The initial browser check used an HTTP request against a SQL endpoint, but the focused local engine tests passed after using the correct in-process path.
⚠️ Additional Finding Medium severity General The hour function returns ERROR 1105 for invalid date text in both SQL modes. The equivalent date cast returns NULL and warning 1292, which is the behavior expected by the integration check.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟡 Malformed hour input returns a hard error
  • Severity: Medium Medium severity
  • Description: The hour function returns ERROR 1105 for invalid date text in both SQL modes. The equivalent date cast returns NULL and warning 1292, which is the behavior expected by the integration check.
  • Impact: Queries that pass invalid date text to the HOUR function fail instead of returning NULL with the expected warning. This can break affected SQL workflows, but valid date queries are not affected.
  • Steps to Reproduce:
    1. Connect to the local MySQL wire endpoint and use the default SQL mode.
    2. Run SELECT HOUR('not-a-temporal-value'); and inspect the result and error.
    3. Compare it with SELECT CAST('not-a-temporal-value' AS DATETIME); followed by SHOW WARNINGS;.
    4. Set the session SQL mode to an empty string and repeat both queries.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: In sql/expression/function/time.go:357-371, Hour.Eval first evaluates the child and tries types.Time.ConvertToTimespan. When that conversion fails, it calls types.DatetimeMaxPrecision.Convert(ctx, val) at lines 366-366. Any conversion error is returned directly at lines 367-368, without calling ctx.Warn and without converting the value to NULL. This bypasses the warning contract used by CAST. In sql/expression/convert.go:265-286, Convert.Eval calls convertValue and, for non-JSON conversion errors, records mysql.ERTruncatedWrongValue through ctx.Warn at lines 276-283 before returning nil. The local MySQL-wire evidence reproduces the mismatch in both the default SQL mode and an empty SQL mode: HOUR returns ERROR 1105 with no warning, while the equivalent DATETIME CAST returns NULL with warning 1292. The PR diff changes the HOUR conversion setup at sql/expression/function/time.go:358-368, but the old implementation also returned the datetime conversion error directly and did not add warning handling; therefore the defect is code-backed but its introduction by this PR is not established. The smallest practical fix is to handle the datetime conversion error in Hour.Eval like the compatible conversion path: record ERTruncatedWrongValue with ctx.Warn and return nil, while preserving the existing JSON or other special-case behavior if shared conversion helpers require it.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

Comment thread sql/expression/function/time.go
@itoqa

itoqa Bot commented Sep 6, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Report433987588d25d1: 9 test cases ran, 2 fixed ✅, 6 passing ✅, 1 additional finding ⚠️.

Diff Summary

Coverage spans core date/time behavior, including conversion and formatting, time-zone normalization, grouping and indexing consistency, hour extraction, execution-path agreement, and invalid-input warnings. It includes normal business-logic flows plus boundary and malformed-input cases, with one unrelated edge-case inconsistency identified for follow-up.

Safe to merge — the only failure is a medium-severity additional finding explicitly not attributable to this PR, with no regressions, new failures, or previously flagged failures remaining. It is a flag for later rather than a merge blocker.

Tests run by Ito

View full run

Result State Severity Type Description
❌->✅ Fixed General Normal, prepared, and generated execution all returned 0 and preserved warning code 1292 with the same diagnostic for the malformed datetime value. The browser error came from opening a local MySQL protocol endpoint as HTTP, not from the SQL behavior under test.
❌->✅ Fixed Warnings Invalid date and time values return NULL with warning 1292, while valid conversions keep their expected results.
Passing Assertions The temporal script sets the time zone before inserting data and checks the expected grouped timestamp results successfully. The browser check could not load the local SQL service because it is not an HTTP endpoint, but the intended native test passed.
Passing General Timezone-bearing rows kept the same normalized times when read directly, found through indexes, and grouped by UNIX timestamp.
Passing General Setting the timezone before or after inserting the rows produced the same stored values and timestamp groups.
Passing Datetime RFC3339 values with fractional seconds were accepted and converted correctly for DATE, DATETIME(3), and TIMESTAMP(3). The results used UTC, removed the time from DATE values, and kept the expected three-digit precision.
Passing Hour The datetime returned hour 20, and the TIME value returned hour 13 as expected.
Passing Sqlmode The database used UTC before saving the temporal rows, and the grouped timestamp results matched all expected values.
⏸️ Skipped Arithmetic Adding two hours to 13:00 produces 15:00, and the numeric expressions return 42, 2, and 1 as expected.
⏸️ Skipped General NULL input stayed NULL, malformed text and binary input returned a repeatable error, and the same malformed query produced the same diagnostic when repeated.
⏸️ Skipped General HOUR returned the expected values at one-hour boundaries, for negative times, for a datetime, and for NULL.
⏸️ Skipped General Explicit and default temporal values were stored in the expected format. Valid updates worked, invalid inserts and updates were rejected without changing existing rows, and automatic timestamp updates worked as expected.
⏸️ Skipped General The first script correctly rejected malformed time text. After changing it to a valid TIME value, the retry returned hour 13 with no warnings, matching a fresh successful run.
⏸️ Skipped Datetime Verified acceptable by independent adversarial review: the reported expectation does not match what the code actually promises. Review notes: The finding conflates support for the TIMESTAMP data type and TIMESTAMP(value) function with TIMESTAMP being a legal MySQL CAST target. Repository code and compatibility tests consistently expose timestamp conversion through TIMESTAMP(...) while the CAST target vocabulary omits TIMESTAMP, so a parser syntax error for CAST(... AS TIMESTAMP) is expected rather than a defect. The finding's separate p…
⏸️ Skipped Harness The normal and prepared query checks both completed successfully. Malformed time input followed the expected error path without hiding or leaving behind a query result.
⏸️ Skipped Mode Valid date and date-time changes were saved with the expected normalized values. An invalid change was rejected, and the existing row stayed unchanged.
⏸️ Skipped Mode The session reported the expected default SQL mode, and valid date and time values were stored and read back correctly. The temporal default also filled the omitted column as expected.
⏸️ Skipped Rev HOUR returned 0, 13, 25, 2, and NULL for midnight, a normal time, an extended-hour time, a negative time, and a NULL value.
⚠️ Additional Finding Medium severity General A timestamp with an explicit offset can cross midnight when converted to UTC, but the different SQL types do not agree about that change. The recorded probe returned June 2 for DATE while DATETIME and TIMESTAMP kept the local June 3 wall-clock value for the +02:00 example; the -02:00 example showed the corresponding mismatch on the other side of midnight.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

🟡 Offset dates cross the wrong day
  • Severity: Medium Medium severity
  • Description: A timestamp with an explicit offset can cross midnight when converted to UTC, but the different SQL types do not agree about that change. The recorded probe returned June 2 for DATE while DATETIME and TIMESTAMP kept the local June 3 wall-clock value for the +02:00 example; the -02:00 example showed the corresponding mismatch on the other side of midnight.
  • Impact: Date and time values with offsets can be shown or compared as the wrong day when they are near midnight. This can lead to incorrect reports or time-based results for affected records.
  • Steps to Reproduce:
    1. Set the session time zone to UTC.
    2. Convert 2010-06-03T00:00:00.000001+02:00 to DATE, DATETIME, and TIMESTAMP, then evaluate UNIX_TIMESTAMP for the same literal.
    3. Repeat with 2010-06-03T23:59:59.999999-02:00 and compare the calendar dates, displayed times, and timestamp values.
    4. Check whether every result represents the same UTC instant and date boundary.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: sql/types/datetime.go:202-209 dispatches string and []byte inputs to parseDatetime, while the only explicit UTC normalization in datetimeType.Convert is value.UTC() at lines 217-218 for an input already typed as time.Time. sql/types/datetime.go:377-385 calls time.Parse for ExtraDatetimeLayouts and returns that time.Time unchanged, so an RFC3339 string retains its numeric-offset location. After parsing, lines 302-325 apply DATE truncation or DATETIME/TIMESTAMP rounding to that unnormalized value. This makes a string literal and an equivalent time.Time value follow different normalization paths. The runtime probe reproduced the consequence at the UTC day boundary: DATE changed calendar day according to the offset, while DATETIME/TIMESTAMP retained the offset-local wall-clock representation; UNIX_TIMESTAMP then reconverted the value through sql/expression/function/date.go:287-321 and could produce a key for a different instant than the displayed conversion. The smallest practical fix is to normalize the successful result from parseDatetimeExtraLayouts, or the string result immediately after parseDatetime, with UTC before target-type truncation and rounding, then add offset-bearing midnight tests for all affected target types.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

Comment thread sql/errors.go Outdated
Comment thread sql/types/year.go
Comment thread sql/types/number.go Outdated
Comment thread sql/types/decimal.go
Comment thread sql/types/datetime.go
Comment thread sql/types/datetime.go
Comment thread sql/expression/convert.go Outdated
Comment thread sql/expression/function/time.go
Comment thread sql/expression/function/json/json_object.go Outdated
@itoqa

itoqa Bot commented Sep 8, 2026

Copy link
Copy Markdown

Ito QA test results
Ito Diff Report88d25d1dfb2ee6: 10 test cases ran, 1 fixed ✅, 8 passing ✅, 1 additional finding ⚠️.

Diff Summary

Coverage spans normal date, time, and timestamp conversion; time extraction and query behavior; timezone handling; zero and malformed values; warning generation; and atomic multi-row writes. It includes happy paths, boundary and invalid-input cases, persistence and grouping behavior, and adversarial checks of error handling and diagnostics.

Safe to merge — the run shows no PR-attributable regressions or unresolved failures, and the exercised conversion, query, warning, timezone, and write behaviors remain healthy. The only finding is an unrelated, minor diagnostic-formatting defect that affects warning clarity but not query results or warning codes and is suitable for later follow-up.

Tests run by Ito

View full run

Result State Severity Type Description
❌->✅ Fixed General Verified acceptable by independent adversarial review: the reported expectation does not match what the code actually promises. Review notes: The core requirement is wrong. A +0200 timestamp near midnight moves earlier, not into the next UTC day, and the reported June 16 versus June 15 contrast exactly matches the code's documented DATETIME(0) versus DATETIME(6) rounding behavior. Moreover, MySQL DATETIME is represented here as a date-and-time value rather than a UNIX timestamp (sql/types/datetime.go:109-120), so the finding does not es…
Passing General Verified acceptable by independent adversarial review: the observed behavior is intended and documented in this codebase. Review notes: The behavior is reachable and accurately described, but the claimed requirement to reject these values is contrary to the implementation's explicit truncation contract and dedicated regression expectations. Warning-plus-prefix conversion is intentional for these exact input classes; the suite separately calls out only particular delimiter forms where this differs from MySQL, and the PR preserves t…
Passing General A multi-row insert with an invalid datetime was rejected, and no earlier row was left in the table. The strict and non-strict checks and the existing insert regression tests all passed.
Passing General HOUR returned 2 for both negative and positive two-and-a-half-hour values, 13 for a datetime, and NULL for NULL. Invalid text returned the expected conversion error.
Passing Datetime A supported datetime string was accepted and returned as 2024-06-15 13:14:15 without an error.
Passing Hour Running HOUR on '2024-06-15 13:14:15' returned 13, which is the expected hour.
Passing Mode The SQL engine stored the all-zero date and datetime, while rejecting dates with a zero month or zero day.
Passing Query Invalid DATE and DATETIME comparisons returned the expected incorrect-value errors, so the query error checks passed.
Passing Warning Passing an invalid date to the DATE function returned NULL and produced MySQL warning 1292 with an incorrect date value message.
⏸️ Skipped Assertions The temporal script sets the time zone before inserting data and checks the expected grouped timestamp results successfully. The browser check could not load the local SQL service because it is not an HTTP endpoint, but the intended native test passed.
⏸️ Skipped General Timezone-bearing rows kept the same normalized times when read directly, found through indexes, and grouped by UNIX timestamp.
⏸️ Skipped General Normal, prepared, and generated execution all returned 0 and preserved warning code 1292 with the same diagnostic for the malformed datetime value. The browser error came from opening a local MySQL protocol endpoint as HTTP, not from the SQL behavior under test.
⏸️ Skipped General Setting the timezone before or after inserting the rows produced the same stored values and timestamp groups.
⏸️ Skipped Datetime RFC3339 values with fractional seconds were accepted and converted correctly for DATE, DATETIME(3), and TIMESTAMP(3). The results used UTC, removed the time from DATE values, and kept the expected three-digit precision.
⏸️ Skipped Hour The datetime returned hour 20, and the TIME value returned hour 13 as expected.
⏸️ Skipped Sqlmode The database used UTC before saving the temporal rows, and the grouped timestamp results matched all expected values.
⏸️ Skipped Warnings Invalid date and time values return NULL with warning 1292, while valid conversions keep their expected results.
⚠️ Additional Finding Minor severity Rev The invalid temporal inputs produced NULL and warning code 1292, but the warning text was malformed and included %!v(MISSING). This happened for the malformed time and zero-date cases, so the warning did not clearly identify the input value.
Additional Findings Details

These findings are unrelated to the current changes but were observed during testing.

⚪ Temporal warnings show broken error text
  • Severity: Minor Minor severity
  • Description: The invalid temporal inputs produced NULL and warning code 1292, but the warning text was malformed and included %!v(MISSING). This happened for the malformed time and zero-date cases, so the warning did not clearly identify the input value.
  • Impact: Users and operators lose the value that caused a temporal conversion warning, which makes SQL errors harder to diagnose. The query still returns NULL and the warning code remains 1292, so the main query flow is not blocked.
  • Steps to Reproduce:
    1. Run SELECT TIME_TO_SEC('not a time'), then run SHOW WARNINGS.
    2. Run SELECT WEEKOFYEAR('0000-00-00'), then run SHOW WARNINGS.
    3. Check the warning message text, not only its 1292 code and NULL result.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: sql/errors.go:991-992 defines ErrIncorrectValue with the format Incorrect %s value: '%v', which requires two arguments: a type label and the offending value. Several changed temporal-function call sites pass only one argument. In sql/expression/function/time.go:646 and :766, YEARWEEK and WEEK call sql.ErrIncorrectValue.New(dateVal); in :1590, TIME_TO_SEC calls New(val); and in :1634 and :1642, WEEKOFYEAR also calls New(val). The resulting error string is passed unchanged to ctx.Warn at each site, so Go's formatting produces %!v(MISSING), matching the captured warnings. The PR diff changes the old error-kind identifier to ErrIncorrectValue at these lines but leaves the incorrect argument count unchanged, which is why introduced_by_this_pr is False. The smallest fix is to pass the appropriate type label and value at each call, for example New(types.Time.String(), val) for TIME_TO_SEC and New(types.Datetime.String(), dateVal) for WEEK/YEARWEEK, then add or update warning assertions for the rendered value.
Evidence Package

Tip

Reply with @itoqa to send us feedback on this test run.

Comment thread sql/expression/function/json/json_object.go
@jycor
jycor merged commit 6950c86 into main Sep 8, 2026
7 of 8 checks passed
@jycor
jycor deleted the james/date branch September 8, 2026 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants