[common] Make quoting mean literal in array and row string cast rules - #9769
Draft
LuciferYang wants to merge 6 commits into
Draft
[common] Make quoting mean literal in array and row string cast rules#9769LuciferYang wants to merge 6 commits into
LuciferYang wants to merge 6 commits into
Conversation
StringToArrayCastRule.splitArrayElements and StringToRowCastRule.splitRowFields toggle quote state on a double quote but then still append the character, so the quote characters stay embedded in the parsed token: an ARRAY<STRING> default value like ["a,b", c] silently stores the element "a,b" with literal quotes, and non-string element types fail parsing instead. The map rule already drops its grouping quotes. Skip the append when the quote toggles state, in both rules: quotes group a token across separators and are not part of the value. Assisted-by: GLM-5.3
LuciferYang
marked this pull request as draft
September 13, 2026 03:06
Stripping the quote characters was only half of it. The splitter forgot that a token had been quoted, so quoting could not express the two values it exists for: an empty quoted token vanished (changing an array's element count and breaking a row's field count) and a quoted "null" became SQL NULL instead of the four-character string. Escapes had the same shape of bug in the other direction: the backslash survived into the value, so nothing could be escaped. Both rules now share one splitter that reports whether a token was quoted. A quoted token keeps its inner whitespace and is never read as the null literal; an unquoted empty token is still absent, so a trailing or doubled separator does not invent a value. The map rule is untouched: it splits entries rather than values, so quote state at the entry level does not tell it whether a key or value was quoted, and fixing it needs quote-aware key/value splitting. Co-Authored-By: Claude Code <noreply@anthropic.com>
The splitter removed quote and backslash characters at every bracket depth, so
a nested literal lost its own syntax before the element rule saw it: the inner
separator stopped being protected and [["a,b"], [c]] became a 2-element inner
array, while ARRAY<MAP<..>> and ARRAY<ROW<..>> turned into "Invalid map entry
format" and "Row field count mismatch". Strip them at depth zero and pass them
through verbatim inside a nested literal, tracking quote state at every depth
so a quoted bracket still cannot move the depth.
Two more from the same review:
An escape now marks its token a literal like a quote does, so [\null] is the
string and not SQL NULL.
A field written as whitespace is empty rather than absent. Only a token that
received no character at all is dropped, which keeps [a, ] at one element and
[a,,b] at two while restoring {a, ,b} to three fields instead of failing the
field count.
Co-Authored-By: Claude Code <noreply@anthropic.com>
An empty-but-present field only existed for interior fields, and it handed the empty string to the element cast, which failed the whole array.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
close #9768
The array and row cast rules toggled quote state on
"but still appended the character, so the quotes ended up inside the token.ARRAY<STRING>["a,b", c]produced the element"a,b"with literal quotes, and a non-string element type such asARRAY<INT>["1", 2]failed to parse at all.Stripping the quotes is only half of it. The splitter also forgot that a token had been quoted, and quoting is precisely how the two values that are otherwise unrepresentable get written:
["", a]dropped the empty token, so the array came back with one element instead of two. For a row that shows up as a field count mismatch.[null, "null"]read both as SQL NULL, so the four-character stringnullcould not be expressed.Escapes were broken in the mirror-image way: the
\\branch set the escape flag but fell through to the append, so the backslash survived into the value and[a\,b]produceda\,b. Nothing could be escaped.Both rules now share one
TokenSplitterthat reports whether a token was quoted. A quoted token keeps its inner whitespace and is never read as the null literal. The two rules previously carried byte-identical copies of the splitter, which is how their semantics drifted, so this collapses them into one.Which level owns a quote
Quotes and backslashes are the syntax of the level that wrote them, so the splitter removes them at bracket depth zero and leaves them alone at depth one or more, where the element's own cast rule parses them again. Without that,
ARRAY<ARRAY<STRING>>[["a,b"], [c]]loses the quotes protecting the inner comma before the inner rule ever runs, and the first element splits into two.The consequence worth knowing: an element that contains a bracket keeps its own quotes and escapes even when its type is scalar, because the splitter decides by depth and not by type.
ARRAY<STRING>[{"a": 1}, b]keeps{"a": 1}intact, where before it came back as{a: 1}. For a row, whose fields do not share one type, deciding by type would mean the splitter knowing each field's type before it has split the fields.What counts as a token
Whitespace outside quotes is not part of a token, so a token made only of whitespace was never written, wherever it sits.
[1,,3]and[1, , 3]are both two elements, and{a,,b}and{a, , b}are both a field count mismatch against a three-field row. Writing an empty value is what quoting is for:{a, "", b}is three fields. An earlier revision made an interior whitespace-only field mean the empty string instead, which read[1, , 3]as[1, "", 3]and failed the whole array onInput is empty., and left the first and last field behaving the other way because the caller trims the body before splitting it.StringToMapCastRuleis deliberately untouched. It splits entries rather than values and then separates key from value with a regex over the already-stripped text, so quote state at the entry level does not tell it whether a key or value was quoted. That needs quote-aware key/value splitting, filed as #9782.Tests
CastExecutorTest.testStringToArrayQuotingAndEscapingandtestStringToRowQuotingAndEscapingcover, for both rules: a quoted separator staying inside one token, a quoted empty token remaining a value, a quotednullstaying a string while an unquoted one becomes SQL NULL, and a backslash escaping a separator without surviving into the value.testStringToNestedArrayKeepsInnerSyntaxcovers the depth rule onARRAY<ARRAY<STRING>>: a quoted and an escaped inner comma, an inner"null", an inner"", inner padding inside quotes, and a plain nested array.testStringToArraySkipsAnEmptyElementandtestStringToRowWhitespaceOnlyFieldIsNotAFieldpin the token rule, including that the first, middle and last position behave alike.Verified on JDK 11: 46 tests in
CastExecutorTestand 70 in thecastingpackage pass, andpaimon-commonbuilds clean with checkstyle and spotless enabled. Fail-on-base was run, not assumed: against the empty-field revision the array test errors withCannot parse '[1, , 3]' as ARRAY: For input string: ''. Input is empty.and the row test reports the missing mismatch.