mysql/json: read numbers the way MySQL does by arthurschreiber · Pull Request #20722 · vitessio/vitess

arthurschreiber · 2026-07-28T00:34:53Z

Description

MySQL and Vitess did not agree on which JSON documents exist. Vitess parsed numbers MySQL refuses, so the same expression could be answered in vtgate and rejected in MySQL, and the answer a query got depended on where it ran. This brings the parser to MySQL's rules, in four parts.

A number too big for a double. MySQL decides how a JSON number will be stored when it parses the document: an integer that fits stays exact, and everything else becomes a double. A number too large for a double has nowhere to live, so MySQL calls the whole document invalid rather than keeping it at the precision it was written to:

SELECT JSON_VALID('1e1025')            -> 0
SELECT JSON_VALID('1e308')             -> 1
SELECT JSON_TYPE('1e1025')             -> ERROR 3141: Invalid JSON text:
                                          "Number too big to be stored in double." at position 0
INSERT INTO t (doc) VALUES ('1e1025')  -> ERROR 3140: same, in value for column 't.doc'

Underflow is not rejected, also matching MySQL — 1e-1000 is a valid document that reads as zero.

The digits go into the significand before the exponent is applied, so what settles this is how a number was written and not what it is worth. A number written to more digits than a double has room for is refused even where a negative exponent would bring it back inside:

SELECT JSON_VALID(CONCAT('1', REPEAT('0', 400)))           -> 0
SELECT JSON_VALID(CONCAT('1', REPEAT('0', 400), 'e-400'))  -> 0   -- worth one
SELECT JSON_VALID(CONCAT(REPEAT('9', 400), 'e-100'))       -> 0
SELECT JSON_VALID(CONCAT('1', REPEAT('0', 307), 'e-400'))  -> 1   -- 308 digits, room to spare

An exponent too big as written. MySQL bounds the exponent before it converts anything. The decimal point may travel 308 places, plus one more for every digit the number already carries after it, so 0e309 is refused even though it is zero while 0.00e310 is fine — its two fraction digits buy the places back:

accepted: 0e308   0.0e309   0.00e310   0.1e309   0.01e310
rejected: 0e309   0.0e310   0.00e311   0.1e310   0.01e311

Neither rule subsumes the other. 0.00e311 converts to zero and is caught only as written; 10e308 is written inside the bound and overflows only once converted.

Numbers written in shapes JSON does not have. The number reader was a general-purpose one rather than a JSON one. It took a written plus, an integer part opening with a zero, and a decimal point with nothing on one side of it — none of which JSON allows and none of which MySQL accepts:

                     vitess (before)   mysql
{"a": .2}            0.2               ERROR 3141
[007]                [007]             ERROR 3141
12.                  12.0              ERROR 3141
+1                   1.0               ERROR 3141

This is the part already causing query failures in the field: whether a JSON expression is evaluated in vtgate or pushed down to MySQL decides whether it answers or errors.

nan. The parser carried a branch for it, inherited from the JSON reader this code grew out of. It is an extension to JSON rather than part of it, and JSON_VALID('nan') is 0.

Attribution. The magnitude check reproduces what RapidJSON's number reader decides, because that is the reader MySQL parses JSON with, and matching it is the whole point. No RapidJSON source is copied — the Go was written to land on the same answers — but reproducing one function's decisions step for step is close enough that its notice belongs in the tree either way, so this adds go/mysql/json/LICENSE.rapidjson and a copyright line to parser.go. That follows what the package already does for fastjson, which it is derived from.

Related Issue(s)

Fixes #20724. Related to #20720.

#20724 closes here: nan was raised against the combined #20723/#20718 code, and the parser no longer reads it as a number.

#20720 stays open, because two of the things it asks for are not the parser's to give.

The first is that no comparison-invalid number reaches execution. This gets most of the way — a number too big for a double no longer parses at all — but a document can still be written inside the bound and carry a number decimal.NewFromString refuses. 0. followed by 800 zeros and 1e1100 is one: its 800 fraction digits buy back its written exponent, so the bound accepts it, and its double is an ordinary 1e299. MySQL compares it against 1e299 and says equal; Vitess answers DECIMAL value is out of range in the interpreter while the compiled path reads a fingerprint instead. So the divergence that prompted the issue is narrowed here rather than closed, and what closes it is #20718, which compares the double MySQL stored instead of the text the number was written as. Underflow is the same story from the other end: 1e-1025 parses, and MySQL reads it as zero, but comparing its written form errors the same way. @mattlord raised that one in review here.

The second is sqltypes.NewJSON, which validates with encoding/json and so still accepts these documents. That one cannot use the JSON parser: go/mysql/json already imports go/sqltypes, so the dependency would be a cycle. It is only reached from test helpers today, and a value built through it is parsed — and now rejected — as soon as it reaches the evalengine through NewFromSQL.

Backport justification

These are wrong answers and spurious failures on released versions, not new behaviour.

The grammar divergence is already breaking queries in production: a JSON expression over a document containing something like .2 or 007 succeeds when vtgate evaluates it and fails when the same query is pushed down to MySQL, so whether a query works depends on planning decisions the user has no control over.

The magnitude and nan divergences give two different answers for one expression. With a JSON column holding 1e1025, column0 IN (CAST('1e1025' AS JSON)) errors in the interpreter — the decimal conversion is out of range — and returns 1 in the compiled form, which reads the fingerprint instead. Which one runs is not a user-visible choice either.

The change is contained to the JSON number reader, and everything it now rejects is a document MySQL would never have accepted in the first place, so nothing that can be stored in a MySQL JSON column is affected.

Verification against MySQL

Checked differentially against a live MySQL 8.0.46 through the evalengine/integration harness, comparing accept/reject on 874 documents: the magnitude boundary walked one step at a time across five fraction widths and both signs, long-fraction forms where the exponent and the fraction are both far out of range but the value lands back inside it, every grammar shape above, nan in four spellings and nested in arrays and objects, and 500 randomly spelled numbers between 1e290 and 1e340 free to emit leading zeros and written plusses. No mismatches.

That walk varies the exponent and the fraction width but keeps the digit string short, which leaves out the numbers written to more digits than a double holds. A second sweep of 5566 documents covers those: digit counts from 305 to 312, the same boundary walks, and 5000 randomly spelled numbers carrying up to 340 digits on either side of the decimal point with exponents out to ±700, so that long digit strings and compensating negative exponents are crossed with each other. No mismatches. That sweep was run against 8.0.46 only.

What this does not cover

Whether a document exists and what its numbers are worth are two questions, and this answers the first one only.

@GrahamCampbell found the case that settled how the first had to be answered. At the top of the range MySQL's verdict turns on how a number is spelled rather than on what it is worth — the same value accepted or rejected according to a trailing zero:

                            mysql      vitess
1.7976931348623157e308      accepted   accepted
1.7976931348623158e308      rejected   rejected
1.79769313486231580e308     accepted   accepted
17976931348623158e292       rejected   rejected
179769313486231580e291      accepted   accepted

No comparison of a correctly converted value against the largest double can produce that, which is why mysqlNumberFits transcribes RapidJSON's conversion rather than doing its own: where the digits stop being a significand and start being a power of ten to scale it by is what decides these, so landing the split where RapidJSON lands it lands the verdict too. TestParseNumberTooBigForDouble pins all five, along with the spellings either side of them.

What is left over is the value. Vitess reads a number with fastparse, which is correctly rounded, while MySQL keeps whatever its approximate conversion arrived at — reading 1.7976931348623157081e308 back gives 1.7976931348623155e308, two ULP low. That is not confined to the boundary:

input shape                                    differs from correctly-rounded
15 significant digits, exponent within ±20      0 / 150
15 significant digits, large exponent          31 / 150
25 significant digits, exponent within ±20     80 / 150
25 significant digits, large exponent          91 / 150
plain decimals, e.g. 291.276103743955106997454 16 / 150

So Vitess still reads a different double than MySQL stores for a large share of JSON numbers carrying more than 15 significant digits. Closing that means having the parser return the value its conversion arrived at and not only its verdict, which is what #20726 does, further up this stack.

Performance

Reading JSON's grammar directly turns out to be less work than the loop that read around it — the leading-zero bookkeeping and the labelled switch both go away — and for a number that never reaches the magnitude check it more than pays for the two new ones. A number that does reach it pays about twice over. Against the merge base, twenty runs a side:

parse                    before      after
int/1024                 14.14µs     8.26µs    -41.6%  (p=0.000, n=20)
frac/1024                17.07µs     9.38µs    -45.1%  (p=0.000, n=20)
exp/1024                 19.66µs    11.98µs    -39.1%  (p=0.000, n=20)
mixed-object              140.5n     124.3n    -11.5%  (p=0.000, n=20)
checked/1024             21.72µs    41.13µs    +89.4%  (p=0.000, n=20)
checked-long-fraction     475.5n     949.9n    +99.8%  (p=0.000, n=20)
rejected                       -      1.66µs

BenchmarkParse in parser_bench_test.go produces these. The first four are arrays of a thousand numbers written three ways and a small mixed object; the last three are the shapes that reach the magnitude check.

That is what the check costs when a number reaches it, and until @GrahamCampbell's review the benchmark did not show it: an earlier version of these cases was written before the prefilter counted leading digits rather than the whole written string, and afterwards a number with one integer digit and a large exponent answered from its digits alone. All three accepted checked cases had stopped reaching the conversion, so they went on measuring the scan and reported a speed-up — the -11.7% and -33.8% an earlier revision of this section claimed for them. reachesMagnitudeCheck now holds every case to what its name says, so a document that stops short of the check fails the benchmark instead of quietly flattering it. All the rows above were re-measured together afterwards, so they differ slightly from that revision's throughout.

What keeps the check off the path an ordinary number takes is asking only what can put a number past the largest double: the digits in front of its decimal point, moved by its exponent. Measuring the whole written number instead counts its fraction, its e and its exponent's own digits as well, which is enough to send a 1.5e303 off to be converted to find out what its four digits already said. readFloat reports the exponent it scanned so that nothing has to look for it again, and the digits are counted only once the cheaper over-count says they might matter.

Reaching the check at all means being written to more digits than a double has room for — over 308 of them, less whatever a positive exponent takes away. checked/1024 is twenty digits scaled by e289 and checked-long-fraction a 309-digit integer with a 400-digit fraction; nothing shorter gets there, and the numbers a document usually carries are three orders of magnitude short of it.

Those two rows are a property of the documents rather than of the check, and measuring the same shapes either side of the prefilter says so — identical numbers, one spelling that reaches the conversion and one that answers from its digits alone:

number                doesn't reach   reaches   check costs
1.5e308                       9.9n     17.2n         +7.3n
twenty digits, e289          14.6n     41.7n          +27n
309-digit integer             109n      659n         +550n

A few nanoseconds fixed and about 1.5n per written digit, which is roughly what a dependent multiply-add chain costs, so there is no fat in it. That is also why checked/1024 reads as +89%: it is a thousand numbers each written to twenty digits and each landing in [1e308, 1e309). A number that plausibly reaches the check is short and pays 7n for it, against the 39-45% saved on every ordinary number sharing the document with it.

Two savings are left unclaimed, both no-ops rather than approximations: the integer loop goes on accumulating after the significand is already infinite, and the fraction loop goes on iterating after seventeen significant digits with its body switched off — 400 fraction digits are about 290n of checked-long-fraction's 950n. Taking them would help the 300-to-700-digit shapes by roughly a fifth and the short ones not at all, in exchange for more branches inside the one function that has to land on RapidJSON's answer decision for decision. A divergence there is the bug this change exists to close, so they are not worth it at that size. Keeping more numbers off the conversion altogether would mean a sharper prefilter — comparing leading digits against 1.797 with a margin wide enough that the conversion's last-place wobble cannot reach it — which is worth doing if a workload ever asks for it and not before.

rejected has no before: that document parsed successfully until this change.

An earlier attempt classified the number eagerly instead, which was much worse — parseNumberType tries ParseInt64 and ParseUint64 before ParseFloat64, so a fractional number pays for two failures (frac/1024 went from 11.5µs to 327µs).

Tests

TestParseNumberTooBigForDouble covers both sides of the magnitude boundary and both sides of the written-exponent bound, underflow, numbers that are long but small, and rejection nested inside arrays and objects. It also covers the spellings called out on the issue — 1e+0, 1e0000000000, 1e-0000000000, 1e-1024, 1e00000000000000000308 — which stay valid: each carries an exponent and so goes through the checks, making them the cases most likely to be rejected by mistake.

It also pins that a negative exponent written to more places than an int holds stays valid and reads as zero, on a number long enough to reach the conversion in the first place: without the stop on the exponent accumulator, the overflow lands on a power of ten the table does not go up to. @GrahamCampbell asked for that one in review.

It also crosses long digit strings with negative exponents in both directions — rejected where the digits run past a double's room (1 and 400 zeros with e-400, 400 nines with e-100), accepted where they do not (1 and 307 zeros with e-400) — since a number can be written well out of range and still be worth very little.

TestParseNumberGrammar covers the four shapes JSON does not have, along with nan, each bare and inside a document.

These are unit tests rather than differential cases on purpose: knownErrors in the integration harness already whitelists MySQL's Invalid JSON text in argument N to function W message, so a differential case passes whether or not Vitess errors. That is also why the harness never caught Vitess accepting these documents, and why the verification above compares accept/reject directly instead.

go/mysql/json, go/mysql/binlog, go/mysql/decimal, go/mysql/datetime, go/sqltypes, go/vt/vtgate/evalengine, the MySQL differential suite, go/vt/vtgate/engine and go/vt/vttablet/tabletmanager/vreplication pass locally. Nothing in the suite depended on any of these documents parsing; the only tests that changed are the three assertions in TestParseRawNumber that pinned the old lax shapes.

Checklist

"Backport to:" labels have been added if this change should be back-ported to release branches
If this change is to be back-ported to previous releases, a justification is included in the PR description
Tests were added or are not required
Did the new or modified tests pass consistently locally and on CI? (locally yes; CI pending)
Documentation was added or is not required

Deployment Notes

JSON documents carrying a number MySQL will not store are now rejected where they used to be accepted: a number too large for a double, a number written to more digits than a double holds even where a negative exponent brings its value back inside, an exponent past what its fraction digits allow, nan, and numbers written with a leading plus, a leading zero, or a decimal point missing digits on one side. Documents already stored are unaffected — none of these could be stored in a MySQL JSON column to begin with — and this is about what Vitess will parse. No migrations or configuration changes.

A release note still needs writing for this.

AI Disclosure

Claude Code wrote this one, including the tests and benchmarks — I reviewed it and provided direction. It came out of reviewing #20691, and grew as @GrahamCampbell found more of the boundary in review.

MySQL decides a JSON number's storage when it parses the document: an integer that fits stays exact and everything else becomes a double. A number too large for a double therefore has nowhere to live, and MySQL calls the whole document invalid — JSON_VALID('1e1025') is 0, and inserting it into a JSON column is refused. Vitess parsed it happily and left every consumer to cope with a value that should not exist, which is how the same expression could error in the interpreter and answer 1 in the compiled form. Underflow is not rejected, matching MySQL: 1e-1000 is a valid document that reads as zero. The check is kept off the common path. readFloat already walks the exponent, so it reports whether one was written, and a number without one has to be longer than the largest double before it can overflow. Parsing arrays of integers and of fractional numbers both measure unchanged. Also sets the number type on the NaN branch, which left whatever the value cache last held in that field. Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

vitess-bot · 2026-07-28T00:35:21Z

A written sign, a zero-padded exponent and an underflowing one are all spellings rather than magnitudes, and all stay valid. They go through the conversion because they carry an exponent, so they are the cases most likely to be caught by mistake. Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

codecov · 2026-07-28T00:49:54Z

Codecov Report

❌ Patch coverage is 99.37107% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.61%. Comparing base (70c7a72) to head (0886b6f).
⚠️ Report is 450 commits behind head on main.

Files with missing lines	Patch %	Lines
go/mysql/json/parser.go	99.36%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #20722       +/-   ##
===========================================
+ Coverage   69.67%   84.61%   +14.94%     
===========================================
  Files        1614       77     -1537     
  Lines      216793    22871   -193922     
===========================================
- Hits       151044    19353   -131691     
+ Misses      65749     3518    -62231

Flag	Coverage Δ
partial	`84.61% <99.37%> (?)`

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

GrahamCampbell · 2026-07-28T02:18:20Z

 	v.s = s[:flen]
 	v.n = numberTypeRaw
+	if mayExceedFloat64(v.s, exponent) {
+		if _, err := fastparse.ParseFloat64(v.s); err != nil {


I think this is still slightly more permissive than MySQL at the upper boundary. ParseFloat64 rounds both of these to math.MaxFloat64, so this branch accepts them:

1.7976931348623158e308 17976931348623158e292

MySQL 8.0.46, 8.4.10, and 9.7.1 reject both as too large. The original spelling also appears significant:

accepted: 1.7976931348623157e308 rejected: 1.7976931348623158e308 accepted: 1.79769313486231580e308

That means comparing only the converted value with MaxFloat64 would not distinguish the neighboring cases. Would it make sense to use a MySQL/RapidJSON-compatible check over the original spelling and pin these boundaries explicitly?

I think 139a987 fixes this.

GrahamCampbell · 2026-07-28T02:18:21Z

 	}

-	flen, ok := readFloat(s)
+	flen, exponent, ok := readFloat(s)


Could readFloat return a bounded exponent and fractional-position adjustment here, rather than reducing the exponent to a boolean? MySQL applies a written positive-exponent limit before conversion, so zero does not make an arbitrarily large exponent valid:

accepted: 0e308 0.0e309 0.00e310 rejected: 0e309 0.0e310 0.00e311

The current ParseFloat64 check accepts every rejected form because each converts to zero without error. MySQL's vendored RapidJSON applies a 308 - expFrac boundary during exponent scanning.

Carrying a saturating exponent plus the fractional adjustment seems like it would reproduce that boundary without overflowing on a very long exponent, while retaining padded spellings such as 1e0000000000.

Fixed in e68bd7e.

GrahamCampbell · 2026-07-28T02:20:20Z

I presume we're backporting this, in this order, because of the dependencies between them:

#20722 -> #20723 -> #20718

A JSON number carries two ways of being too big, and only one of them shows up after conversion. MySQL bounds the exponent a number is written with before it converts anything: the decimal point can travel 308 places, plus one for every digit the number already has after it. 0e309 is refused on those grounds even though it is zero, while 0.00e310 is fine because its two fraction digits buy the places back. Converting caught the second way and missed the first, so every exponent that lands on zero came through: 0e309, 0.0e310, 0.00e311. Converting is still needed for the other direction — 10e308 is written within the bound and overflows anyway. readFloat already walks both the fraction digits and the exponent, so it reports them rather than reporting that an exponent was there at all. A written exponent stops at a ceiling that clears the largest double plus every digit the number could hold after its point, so a long one neither overflows nor changes the answer. Negative exponents are left alone, matching MySQL: underflow is a valid document that reads as zero. Parsing arrays of integers, of fractional numbers and of numbers with exponents all measure unchanged. Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The number reader was a general-purpose one: it took a written plus, an integer part opening with a zero, and a decimal point with nothing on one side of it. JSON allows none of those and MySQL rejects all of them, so Vitess answered questions MySQL refuses to answer — JSON_EXTRACT('{"a": .2}', '$.a') is 0.2 here and an error there. Which one a query gets depends on whether it is evaluated in vtgate or pushed down, so the same query fails in one place and succeeds in the other. A number written 007 also kept its leading zeros all the way back out again. Reading the grammar directly turns out to be less work than the loop that read around it: the leading-zero bookkeeping and the labelled switch go away, and parsing an array of integers or of fractional numbers is around a fifth to a quarter faster. Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3010e5588

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-28T15:21:51Z

+	if mayExceedFloat64(v.s, exponent) && !mysqlNumberFits(v.s) {
+		return nil, s, fmt.Errorf("number too big to be stored in double: %q", v.s)
+	}


Keep MySQL's double limit out of generic JSON parsing

On MariaDB-backed clusters, JSON is validated text, so a syntactically valid document such as 1e309 can be stored and accepted by MariaDB; however, the evalengine paths call this generic parser without a server-flavor parameter, and this check now rejects that value whenever VTGate evaluates the expression locally. This recreates plan-dependent failures for MariaDB, so the magnitude restriction needs to be selected by flavor rather than applied unconditionally.

AGENTS.md reference: AGENTS.md:L227-L229

Useful? React with 👍 / 👎.

MariaDB is not really supported as a backing cluster, so this is not relevant.

chatgpt-codex-connector · 2026-07-28T15:21:51Z

 	v.s = s[:flen]
 	v.n = numberTypeRaw
+	if mayExceedFloat64(v.s, exponent) && !mysqlNumberFits(v.s) {
+		return nil, s, fmt.Errorf("number too big to be stored in double: %q", v.s)


Truncate oversized numbers in parser errors

When an untrusted JSON document contains a syntactically valid multi-megabyte number that exceeds this limit, formatting v.s with %q copies the entire token into the inner error, and Parser.Parse then copies that message again while wrapping it. These inputs previously parsed without this error allocation, so a request near the packet-size limit can now cause substantial transient memory amplification and an equally oversized error response; use the existing startEndString truncation for this diagnostic.

Useful? React with 👍 / 👎.

Done in a3fb097.

chatgpt-codex-connector · 2026-07-28T15:21:51Z

 	// malformed inputs like "1+2", "1..2", or "1e+" that a simple
 	// character-class loop would accept.
-	n, ok := readFloat(w.data[w.pos:])
+	n, _, ok := readFloat(w.data[w.pos:])


Apply the magnitude check in the raw SQL marshaller

This call discards the exponent returned by readFloat, so AppendMarshalSQL and MarshalSQLValue still accept numbers that the updated parser rejects, such as 1e309 or 1.7976931348623159e308. They emit expressions such as CAST(1e309 as JSON) or JSON_ARRAY(1e309), which MySQL rejects when vreplication executes them; apply the same mayExceedFloat64/mysqlNumberFits validation here so callers fail before constructing an unusable replication statement.

Useful? React with 👍 / 👎.

This is tracked in #20727 and is not really a reachable issue.

arthurschreiber · 2026-07-28T15:29:23Z

I think that we should preserve MySQL’s zero semantics for accepted underflow. In go/mysql/json/parser.go:205, 1e-1025 is correctly accepted, but its original spelling remains in v.s. MySQL converts it to double zero, while Value.Decimal() rejects it because decimal.ExponentLimit is 1024. JSON comparison therefore returns DECIMAL value is out of range instead of comparing it as zero. I think we should preserve MySQL’s converted underflow semantics and add an evalengine regression comparing CAST('1e-1025' AS JSON) with JSON zero.

@mattlord I took a deeper look into this and noticed this is actually fixed by #20718, which is stacked on top of the changes in this PR.

The exponent accumulator is stopped before it overflows the int it reads into, and nothing reached that stop. Past it the exponent lands wherever the overflow leaves it, which can be a power of ten the scaling table does not go up to. Reaching the stop takes more than a long exponent. Counting the digits that can carry a number past a double, rather than the whole written string, means a short number with a long exponent no longer goes through the conversion at all — so these cross an exponent written past what an int holds with a digit string long enough to be converted in the first place. Each stays valid and reads as zero, as it does in MySQL 8.0.46. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Nothing bounds how long a number may be written, and Parse copies the message it wraps, so naming the number in full carried the document into the error twice over. It goes through startEndString instead, as the unparsed tail alongside it already did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3fb097a3c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-28T16:45:13Z

-	flen, ok := readFloat(s)
+	flen, exponent, ok := readFloat(s)
 	if !ok {
 		return nil, s[flen:], fmt.Errorf("invalid number in JSON string: %q", s)


Truncate invalid-number diagnostics

When untrusted input is a multi-megabyte number newly rejected by the stricter grammar, such as + followed by millions of digits, %q copies the entire document into this error and Parser.Parse copies that message again while wrapping it, producing substantial transient memory amplification and an oversized client error. The prior oversized-magnitude diagnostic now uses startEndString, but this grammar-failure path still interpolates all of s; abbreviate it here as well.

Useful? React with 👍 / 👎.

Done in 94366cc.

chatgpt-codex-connector · 2026-07-28T16:45:13Z

+	{name: "checked/1024", doc: numberArray(1024, func(i int) string { return "1." + strconv.Itoa(i) + "e30" + strconv.Itoa(i%8) })},
+	{name: "checked-long-fraction", doc: "0." + strings.Repeat("0", 400) + "1e-400"},


Exercise the accepted magnitude check in benchmarks

Neither accepted checked case reaches mysqlNumberFits as claimed: checked/1024 has one integer digit and exponents 300–307, so digits > 308-exponent is always false, while the negative exponent in checked-long-fraction leaves room at 308 and again counts only one integer digit. Consequently the only benchmark that executes the new conversion is the rejected error path, so regressions in the accepted conversion path remain unmeasured; use accepted documents for which mayExceedFloat64 actually returns true.

AGENTS.md reference: AGENTS.md:L79-L80

Useful? React with 👍 / 👎.

Done in 97ea69c, PR body updated with new benchmark results.

Reading numbers by JSON's grammar turned documents that used to parse into rejections, and deleting the nan branch turned another one into a token error. Both name the text they refuse in full, and Parse copies the message it wraps, so a document that arrives megabytes long comes back as its own error twice over. The magnitude check was abbreviated already; the three remaining sites follow it. One test now covers every shape that reaches them, in place of the single case that covered the magnitude error alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

None of the accepted checked cases reached it. Counting the digits that can carry a number past a double, rather than the whole written string, left a number with one integer digit and a large exponent answering the question from its digits alone — so cases built out of those went on measuring the scan they share with every other case, and read as though the check were free. The two accepted cases are now written to more digits than their exponents leave a double room for, and reachesMagnitudeCheck holds every case to what its name claims, so a document that stops short of the check fails the benchmark rather than quietly flattering it. Reaching the conversion costs what it costs: against the merge base checked/1024 goes from 21.72µs to 41.13µs and checked-long-fraction from 475.5n to 949.9n, where the same names reported a speed-up before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

GrahamCampbell · 2026-07-28T17:35:13Z

I think the “What this does not cover” section is stale now. It still says this PR accepts 1.7976931348623158e308 and leaves the RapidJSON conversion for a follow-up, but the current code rejects it and handles the spelling-dependent boundary here. Should we remove or update that section so the description matches the final implementation?

arthurschreiber · 2026-07-28T17:40:37Z

I think the “What this does not cover” section is stale now. It still says this PR accepts 1.7976931348623158e308 and leaves the RapidJSON conversion for a follow-up, but the current code rejects it and handles the spelling-dependent boundary here. Should we remove or update that section so the description matches the final implementation?

@GrahamCampbell Done! ❤️

AppendMarshalSQL scans documents with the same readFloat the parser uses, so the shapes it stopped accepting are rejected here too. Two of them reach the reader — 007 now stops after the lone zero and fails the trailing-data check, and 12. no longer reads at all — while +1 and .2 never get past writeValue's dispatch, before and after alike. All four are pinned so the two callers' expectations stay together. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

GrahamCampbell

parser_test.go keeps this branch's t.Fatalf style around the widened readFloat return; everything else applied cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

marshal.go and marshal_test.go stay as release-23.0 has them: the upstream change there adapted AppendMarshalSQL's readFloat call, and that writer does not exist on this branch. parser.go takes the upstream readFloat wholesale, and parser_test.go keeps this branch's t.Fatalf style around the widened readFloat return. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

Copilot AI review requested due to automatic review settings July 28, 2026 00:34