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

sql/src/main/scala/app/softnetwork/elastic/sql/parser/Parser.scala

@@ -247,8 +247,72 @@ object Parser
     identifierWithIntervalFunction |
     identifierWithFunction
 
+  /** The `SCRIPT AS ( … )` body, delimited by scanning its own balanced parentheses.
+    *
+    * `scriptValue` cannot be relied on to stop at the wrapper's closing paren: the generic
+    * `identifierWithFunction` closes with `rep1(end)` — one or MORE — so a call to a name-only
+    * extractor (`YEAR(x)`, `MONTH(x)`, `QUARTER(x)`, …) consumed the `)` belonging to the wrapper.
+    * `script` then failed, and a column definition fell through to `optionalMultiFields`, which is
+    * why `age INT SCRIPT AS (YEAR(CURRENT_DATE) - YEAR(birthdate))` — the documented example —
+    * reported `')' expected but 'S' found`. `SELECT YEAR(x)` was never affected: nothing enclosed
+    * it.
+    *
+    * Balancing that shared production is NOT the fix, and three attempts proved it: `rep1sep(
+    * sql_function, start)` is ambiguous, and the greed of `rep1(end)` is what selects the correct
+    * parse for a nested call like `MAX(YEAR(DATE_TRUNC(DATETIME_PARSE(…), MINUTE)))`. Bounding the
+    * close count, or giving the extractors their own balanced production, silently drops `Year`
+    * from `identifier.functions` — while `.sql` still renders it, so only the emitted painless
+    * script shows the loss. Deciding the boundary here instead leaves every other parse untouched.
+    */
+  private def scriptBody: Parser[String] = new Parser[String] {
+    def apply(in: Input): ParseResult[String] = {
+      val source = in.source
+      var i = in.offset
+      while (i < source.length && source.charAt(i).isWhitespace) i += 1
+      // `Error`, not `Failure`: `SCRIPT AS` has already been consumed, so no alternative is
+      // legitimate from here. A `Failure` is discarded by both call sites for structural reasons —
+      // `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 — which left the diagnostic below unreachable and reported the very message
+      // this production exists to eliminate.
+      if (i >= source.length || source.charAt(i) != '(')
+        Error("'(' expected after SCRIPT AS", in)
+      else {
+        val bodyStart = i + 1
+        var depth = 0
+        var quote: Char = 0
+        var closing = -1
+        while (i < source.length && closing < 0) {
+          val c = source.charAt(i)
+          if (quote != 0) {
+            if (c == '\\') i += 1
+            else if (c == quote) quote = 0
+          } else
+            c match {
+              case '\'' | '"' => quote = c
+              case '('        => depth += 1
+              case ')'        => depth -= 1; if (depth == 0) closing = i
+              case _          => ()
+            }
+          i += 1
+        }
+        if (closing < 0) Error("unbalanced parentheses in SCRIPT AS", in)
+        else
+          Success(
+            source.subSequence(bodyStart, closing).toString,
+            in.drop(closing + 1 - in.offset)
+          )
+      }
+    }
+  }
+
   def script: PackratParser[PainlessScript] =
-    (keyword("SCRIPT") ~ keyword("AS")) ~ start ~ scriptValue ~ end ^^ { case _ ~ _ ~ s ~ _ => s }
+    (keyword("SCRIPT") ~ keyword("AS")) ~> scriptBody >> { body =>
+      parseAll(scriptValue, body) match {
+        case Success(s, _)     => success(s)
+        case NoSuccess(msg, _) => err(s"Invalid SCRIPT AS expression ($body): $msg")
+      }
+    }
 
   def column: PackratParser[Column] =
     ident ~ extension_type ~ (script | optionalMultiFields) ~ defaultVal ~ notNull ~ comment ~ (options | success(

sql/src/main/scala/app/softnetwork/elastic/sql/schema/package.scala

@@ -1602,7 +1602,13 @@ package object schema {
                 table.copy(
                   columns = table.columns.map { col =>
                     if (col.name == columnName)
-                      col.copy(script = Some(newScript.copy(dataType = col.dataType)))
+                      // A column is either multi-field or script-defined, never both — the grammar
+                      // says so (`ident ~ extension_type ~ (script | optionalMultiFields)`), so a
+                      // column carrying both renders DDL that cannot be parsed back.
+                      col.copy(
+                        script = Some(newScript.copy(dataType = col.dataType)),
+                        multiFields = Nil
+                      )
                     else col
                   }
                 )
@@ -1737,9 +1743,11 @@ package object schema {
               else {
                 col match {
                   case Some(c) =>
+                    // …and symmetrically: declaring sub-fields drops a script the column had.
                     val updated = c
                       .copy(
-                        multiFields = newFields.toList.map(_.update(Some(c)))
+                        multiFields = newFields.toList.map(_.update(Some(c))),
+                        script = None
                       )
                       .updateStruct()
                     table.copy(
@@ -1766,7 +1774,8 @@ package object schema {
                     val updated = c
                       .copy(
                         multiFields =
-                          c.multiFields.filterNot(_.name == field.name) :+ field.update(Some(c))
+                          c.multiFields.filterNot(_.name == field.name) :+ field.update(Some(c)),
+                        script = None
                       )
                       .updateStruct()
                     table.copy(
@@ -1876,6 +1885,14 @@ package object schema {
           errors = errors :+ s"Partition column ${partition.column} does not exist in table $name"
         }
       }
+      // A column is either multi-field or script-defined, never both: the grammar offers one or the
+      // other (`ident ~ extension_type ~ (script | optionalMultiFields)`), so such a column renders
+      // `name TEXT FIELDS (…) SCRIPT AS (…)`, which cannot be parsed back — and that rendering is
+      // what SHOW CREATE TABLE and the diff statements emit. Loud here beats invalid DDL there.
+      columns.filter(c => c.script.isDefined && c.multiFields.nonEmpty).foreach { c =>
+        errors = errors :+
+          s"Column ${c.name} of table $name cannot declare both FIELDS and SCRIPT AS"
+      }
       if (errors.isEmpty) Right(()) else Left(errors.mkString("\n"))
     }
 

sql/src/test/scala/app/softnetwork/elastic/sql/parser/ScriptFunctionParenSpec.scala

@@ -0,0 +1,114 @@
+package app.softnetwork.elastic.sql.parser
+
+import app.softnetwork.elastic.sql.query.{AlterTable, CreateTable}
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+/** `identifierWithFunction` closed with `rep1(end)` — one or more parentheses — so it consumed a
+  * `)` belonging to whatever production enclosed it. Inside `SCRIPT AS ( … )` that is the wrapper's
+  * own closing paren, which made every name-only extractor function unusable in a generated column:
+  * `age INT SCRIPT AS (YEAR(CURRENT_DATE) - YEAR(birthdate))` — the published example — could not
+  * be parsed, while `SELECT YEAR(x)` always worked because nothing enclosed it.
+  */
+class ScriptFunctionParenSpec extends AnyFlatSpec with Matchers {
+
+  private def scriptOf(sql: String): String =
+    Parser(sql) match {
+      case Right(CreateTable(_, Right(columns), _, _, _, _, _)) =>
+        columns
+          .find(_.script.isDefined)
+          .flatMap(_.script.map(_.sql))
+          .getOrElse(fail(s"no scripted column in [$sql]"))
+      case other => fail(s"Expected a CreateTable, got $other")
+    }
+
+  "the documented CREATE TABLE example" should "parse" in {
+    val sql =
+      """CREATE TABLE users (
+        |  id INT,
+        |  birthdate DATE,
+        |  age INT SCRIPT AS (YEAR(CURRENT_DATE) - YEAR(birthdate)),
+        |  PRIMARY KEY (id)
+        |)""".stripMargin
+    Parser(sql).isRight shouldBe true
+    scriptOf(sql) should include("YEAR")
+  }
+
+  "every date-part extractor" should "be usable inside SCRIPT AS" in {
+    // Name-only extractors — the family that reaches the generic function path.
+    Seq(
+      "YEAR",
+      "MONTH",
+      "DAY",
+      "WEEK",
+      "QUARTER",
+      "EPOCHDAY",
+      "YEARDAY",
+      "HOUR",
+      "MINUTE",
+      "SECOND"
+    )
+      .foreach { fn =>
+        val sql = s"ALTER TABLE users ALTER COLUMN part SET SCRIPT AS ($fn(birthdate))"
+        withClue(s"[$sql] ") { Parser(sql).isRight shouldBe true }
+      }
+  }
+
+  it should "still work for the functions that were never affected" in {
+    Seq(
+      "ALTER TABLE users ALTER COLUMN d SET SCRIPT AS (WEEKDAY(birthdate))",
+      "ALTER TABLE users ALTER COLUMN a SET SCRIPT AS (ABS(salary))",
+      "ALTER TABLE users ALTER COLUMN n SET SCRIPT AS (UPPER(name))",
+      "ALTER TABLE users ALTER COLUMN g SET SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR))",
+      "ALTER TABLE users ALTER COLUMN p SET SCRIPT AS (birthdate + 1)"
+    ).foreach { sql =>
+      withClue(s"[$sql] ") { Parser(sql).isRight shouldBe true }
+    }
+  }
+
+  "a nested function call" should "still consume its own parentheses" in {
+    // The balance has to hold at every depth, in the context where nothing encloses it …
+    // `SELECT ABS(YEAR(x))` is deliberately absent: wrapping an extractor in a math function does
+    // not parse on main either — a separate, pre-existing limitation, not this boundary.
+    Seq(
+      "SELECT YEAR(createdAt) FROM t",
+      "SELECT YEAR(DATE_TRUNC(createdAt, MONTH)) FROM t",
+      "SELECT MAX(YEAR(DATE_TRUNC(createdAt, MINUTE))) AS m FROM t GROUP BY id",
+      "SELECT YEAR(CURRENT_DATE) - YEAR(birthdate) FROM t"
+    ).foreach { sql =>
+      withClue(s"[$sql] ") { Parser(sql).isRight shouldBe true }
+    }
+    // … and where one does.
+    Seq(
+      "ALTER TABLE t ALTER COLUMN c SET SCRIPT AS (YEAR(DATE_TRUNC(createdAt, MONTH)))",
+      "ALTER TABLE t ALTER COLUMN c SET SCRIPT AS (DATE_TRUNC(createdAt, MONTH))"
+    ).foreach { sql =>
+      withClue(s"[$sql] ") { Parser(sql).isRight shouldBe true }
+    }
+  }
+
+  "a malformed script body" should "say so, instead of blaming a parenthesis elsewhere" in {
+    // The scanner's own diagnostics only surface as `Error`: a `Failure` is discarded by both call
+    // sites — `column`'s `script | optionalMultiFields` keeps the alternative's Success, and
+    // `alterTable`'s `repsep` accepts zero statements — which reported `')' expected but 'S'
+    // found`, the exact symptom this production exists to eliminate.
+    val unbalanced = Parser("CREATE TABLE t (a INT SCRIPT AS (b + 1")
+    unbalanced.isLeft shouldBe true
+    unbalanced.swap.toOption.get.msg should include("unbalanced parentheses in SCRIPT AS")
+
+    val noParen = Parser("CREATE TABLE t (a INT SCRIPT AS b)")
+    noParen.isLeft shouldBe true
+    noParen.swap.toOption.get.msg should include("'(' expected after SCRIPT AS")
+
+    val badBody = Parser("ALTER TABLE t ALTER COLUMN a SET SCRIPT AS (@@@)")
+    badBody.isLeft shouldBe true
+    badBody.swap.toOption.get.msg should include("Invalid SCRIPT AS expression")
+  }
+
+  "a scripted column" should "re-parse the SQL it renders" in {
+    val sql = "ALTER TABLE users ALTER COLUMN age SET SCRIPT AS (YEAR(birthdate))"
+    val stmt = Parser(sql).toOption.getOrElse(fail(s"did not parse: $sql"))
+    stmt shouldBe a[AlterTable]
+    Parser(stmt.sql) shouldBe Right(stmt)
+  }
+}

sql/src/test/scala/app/softnetwork/elastic/sql/schema/ColumnScriptFieldsExclusionSpec.scala

@@ -0,0 +1,99 @@
+package app.softnetwork.elastic.sql.schema
+
+import app.softnetwork.elastic.sql.`type`.SQLTypes
+import app.softnetwork.elastic.sql.parser.Parser
+import app.softnetwork.elastic.sql.query.{AlterTable, AlterTableStatement}
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+/** A column is either multi-field or script-defined, never both — the grammar offers one or the
+  * other (`ident ~ extension_type ~ (script | optionalMultiFields)`). `Table.merge` did not honour
+  * that: `ALTER COLUMN … SET SCRIPT AS` kept the existing sub-fields, and the resulting `Table.sql`
+  * — what SHOW CREATE TABLE and the diff statements emit — rendered `name TEXT FIELDS (…) SCRIPT AS
+  * (…)`, which cannot be parsed back.
+  */
+class ColumnScriptFieldsExclusionSpec extends AnyFlatSpec with Matchers {
+
+  private def statementsOf(sql: String): Seq[AlterTableStatement] =
+    Parser(sql) match {
+      case Right(AlterTable(_, _, statements)) => statements
+      case other                               => fail(s"Expected an AlterTable, got $other")
+    }
+
+  private val withFields: Table =
+    Parser("CREATE TABLE t (id INT, name TEXT FIELDS (raw KEYWORD), PRIMARY KEY (id))") match {
+      case Right(ct: app.softnetwork.elastic.sql.query.CreateTable) =>
+        Table(
+          name = ct.table,
+          columns = ct.ddl.getOrElse(fail("expected columns")),
+          primaryKey = ct.primaryKey
+        ).update()
+      case other => fail(s"Expected a CreateTable, got $other")
+    }
+
+  private def columnOf(table: Table, name: String): Column =
+    table.columns.find(_.name == name).getOrElse(fail(s"no column $name"))
+
+  "setting a script" should "drop the sub-fields the column had" in {
+    val merged =
+      withFields.merge(statementsOf("ALTER TABLE t ALTER COLUMN name SET SCRIPT AS (UPPER(name))"))
+    val c = columnOf(merged, "name")
+    c.script shouldBe defined
+    c.multiFields shouldBe empty
+  }
+
+  "declaring sub-fields" should "drop a script the column had" in {
+    val scripted =
+      withFields.merge(statementsOf("ALTER TABLE t ALTER COLUMN name SET SCRIPT AS (UPPER(name))"))
+    val refielded =
+      scripted.merge(statementsOf("ALTER TABLE t ALTER COLUMN name SET FIELDS (raw KEYWORD)"))
+    val c = columnOf(refielded, "name")
+    c.multiFields.map(_.name) should contain("raw")
+    c.script shouldBe empty
+  }
+
+  it should "drop a script when a single field is added" in {
+    val scripted =
+      withFields.merge(statementsOf("ALTER TABLE t ALTER COLUMN name SET SCRIPT AS (UPPER(name))"))
+    val refielded =
+      scripted.merge(statementsOf("ALTER TABLE t ALTER COLUMN name ADD FIELD raw KEYWORD"))
+    columnOf(refielded, "name").script shouldBe empty
+  }
+
+  "the merged table" should "still render DDL that parses" in {
+    val merged =
+      withFields.merge(statementsOf("ALTER TABLE t ALTER COLUMN name SET SCRIPT AS (UPPER(name))"))
+    withClue(s"rendered [${merged.sql}] ") { Parser(merged.sql).isRight shouldBe true }
+  }
+
+  "a table holding an impossible column" should "fail validation rather than render invalid DDL" in {
+    // Reachable outside SQL — `IndexField.ddlColumn` fills `script` and `multiFields` from a live
+    // mapping independently.
+    val impossible = withFields.copy(columns = withFields.columns.map {
+      case c if c.name == "name" =>
+        c.copy(script =
+          columnOf(
+            withFields.merge(
+              statementsOf("ALTER TABLE t ALTER COLUMN name SET SCRIPT AS (UPPER(name))")
+            ),
+            "name"
+          ).script
+        )
+      case c => c
+    })
+    impossible.columns.find(_.name == "name").exists(_.multiFields.nonEmpty) shouldBe true
+    impossible.validate().isLeft shouldBe true
+    impossible.validate().swap.toOption.get should include(
+      "cannot declare both FIELDS and SCRIPT AS"
+    )
+  }
+
+  "a plain column" should "still validate" in {
+    withFields.validate() shouldBe Right(())
+    Table(name = "u", columns = List(Column("id", SQLTypes.Int)))
+      .update()
+      .validate() shouldBe Right(
+      ()
+    )
+  }
+}