fix(sql): stop SCRIPT AS losing its closing parenthesis to a function call

[fupelaqu]

age INT SCRIPT AS (YEAR(CURRENT_DATE) - YEAR(birthdate)) — the example the documentation publishes — could not be parsed, and the error blamed a parenthesis somewhere else: ')' expected but 'S' found, pointing at the SCRIPT keyword.

What was actually wrong

identifierWithFunction closes with rep1(end) — one or more parentheses — so a call to a name-only extractor consumed the ) belonging to whatever enclosed it. Measured by feeding scriptValue the text as script sees it, with the wrapper's paren still pending:

input	left over
YEAR(birthdate))	nothing — both parens eaten
WEEKDAY(birthdate))	) ✓
ABS(salary))	) ✓
DATE_DIFF(birthdate, CURRENT_DATE, YEAR))	) ✓
birthdate + 1)	) ✓

script was SCRIPT AS ~ start ~ scriptValue ~ end; with the closing paren gone its end found nothing, script failed, and a column definition fell through to optionalMultiFields — which is where the misleading message came from.

It hits only the name-only extractors (YEAR, MONTH, DAY, WEEK, QUARTER, EPOCHDAY, YEARDAY, HOUR, MINUTE, SECOND), because those are declared as YEAR.regex ^^ (_ => new Year) and reach the generic path. WEEKDAY escapes it — day_of_week_tr consumes NAME ( ident ) with a single end of its own — as do ABS, UPPER, DATE_DIFF and arithmetic. SELECT YEAR(x) was never affected: nothing encloses it there. The documented SQL was correct all along.

Why the fix is where it is

Balancing the shared production is not the fix, and three attempts proved it on this fixture:

MAX(year(date_trunc(datetime_parse(createdAt, '%Y-%m-%d %H:%i:%s.%f'), MINUTE)))

Bounding the close count with repN, adding a balanced extractor_identifier, and removing extractor_function from sql_function each produced the same regression: identifier.functions lost Year (and MAX$ became MaxAgg) while .sql still rendered MAX(YEAR(DATE_TRUNC(…))). The only visible symptom was the emitted painless script losing .get(ChronoField.YEAR) — the sql suite stayed green at 487 and only the bridge suite caught it. rep1sep(sql_function, start) is ambiguous, and the greed of rep1(end) is what selects the correct nested parse; any change to the paren accounting reshuffles the winner.

So the boundary is decided locally instead. A new scriptBody scans the balanced, quote- and escape-aware parenthesised body, and script re-parses just that text with parseAll(scriptValue, …). scriptValue never sees the outer paren, and every other production is untouched. Both call sites benefit, since alterColumnScript delegates to script.

Diagnostics

The scanner reports Error, not Failure. SCRIPT AS is already consumed, so no alternative is legitimate — and a Failure is discarded by both call sites: column's script | optionalMultiFields keeps the alternative's Success and drops the deeper failure, and alterTable's repsep accepts zero statements so phrase overwrites the message. That left the scanner's own message unreachable and still reported the symptom this PR exists to remove. Now:

input	before	after
CREATE TABLE t (a INT SCRIPT AS (b + 1	')' expected but 'S' found	unbalanced parentheses in SCRIPT AS
CREATE TABLE t (a INT SCRIPT AS b)	')' expected but 'S' found	'(' expected after SCRIPT AS
ALTER … SET SCRIPT AS (@@@)	end of input expected	Invalid SCRIPT AS expression (@@@): …

Verification

sql 488 on Scala 2.12 and 2.13 · core 737 · macrosTests 18 · bridge template and all four es{6,7,8,9} bridges 120 each (es9 under sbt17) · scalafmtCheckAll + headerCheck clean.

Four of the six new tests fail without the fix; the "functions that were never affected" case stays green, as it must.

Also in this PR: a column is multi-field or script-defined, never both

Project decision, and the grammar already said it — column is ident ~ extension_type ~ (script | optionalMultiFields). Table.merge did not honour it: ALTER COLUMN … SET SCRIPT AS kept the sub-fields the column already had, so Table.sql rendered name TEXT FIELDS (…) SCRIPT AS (…), which cannot be parsed back — and that rendering is what SHOW CREATE TABLE and the diff statements emit. Reachable from plain SQL:

CREATE TABLE t (name TEXT FIELDS (raw KEYWORD));
ALTER TABLE t ALTER COLUMN name SET SCRIPT AS (UPPER(name));

Setting a script now clears the sub-fields, and declaring sub-fields (SET FIELDS / ADD FIELD) clears the script. Table.validate rejects a column holding both, because the SQL path is not the only way to build one — IndexField.ddlColumn fills script and multiFields independently from a live mapping, and failing loudly there beats emitting DDL that will not parse.

Not fixed here

• SELECT ABS(YEAR(x)) — wrapping a date-part extractor in a scalar function does not parse, on main too. Same unbalanced rep1(end) as above, in the general case rather than the SCRIPT AS one. Filed as Scalar function wrapping a date-part extractor does not parse: SELECT ABS(YEAR(x)) #220 with the measured boundary (aggregates work, scalars do not) and a warning about the three fixes that regress the nested fixture.
• SCRIPT AS () reports an inner-alternation message (… '(?i)(LEAST)\b' expected …) rather than "empty script body".

🤖 Generated with Claude Code