fix(sql): make ALTER COLUMN ... SET|ADD FIELD parse and actually apply

core/src/main/resources/help/commands/ddl/alter_table.json

@@ -23,7 +23,7 @@
     "ALTER COLUMN [IF EXISTS] column_name DROP NOT NULL",
     "ALTER COLUMN [IF EXISTS] column_name SET COMMENT 'text'",
     "ALTER COLUMN [IF EXISTS] column_name DROP COMMENT",
-    "ALTER COLUMN [IF EXISTS] column_name SET TYPE new_type",
+    "ALTER COLUMN [IF EXISTS] column_name SET DATA TYPE new_type",
     "ALTER COLUMN [IF EXISTS] column_name SET SCRIPT AS (expression)",
     "ALTER COLUMN [IF EXISTS] column_name DROP SCRIPT",
     "ALTER COLUMN [IF EXISTS] column_name SET OPTIONS (option = value, ...)",
@@ -87,7 +87,7 @@
       "optional": false
     },
     {
-      "name": "SET TYPE",
+      "name": "SET DATA TYPE",
       "description": "Change column data type (requires reindex)",
       "optional": false
     },

documentation/sql/ddl_statements.md

@@ -307,6 +307,32 @@ CREATE TABLE users (
 PARTITIONED BY (birthdate MONTH);
 ```
 
+### Table Options (index settings)
+
+Index settings are declared with a table-level `OPTIONS (…)` clause after the column list:
+
+```sql
+CREATE TABLE users (
+  id INT,
+  name VARCHAR,
+  PRIMARY KEY (id)
+) OPTIONS (number_of_shards = 1, number_of_replicas = 0);
+```
+
+Any index setting can be passed — for example `default_pipeline` to route inserts through a
+custom ingest pipeline (see [Using Enrich Policies in Pipelines](#using-enrich-policies-in-pipelines)):
+
+```sql
+CREATE TABLE events (
+  id INT,
+  user_id KEYWORD,
+  PRIMARY KEY (id)
+) OPTIONS (default_pipeline = 'events_enriched');
+```
+
+> ⚠️ `OPTIONS`, not `WITH` — `WITH (…)` belongs to `CREATE MATERIALIZED VIEW`. A
+> `CREATE TABLE … WITH (…)` is rejected by the parser (the whole statement must parse).
+
 ---
 
 ## CREATE TABLE AS SELECT
@@ -346,10 +372,32 @@ The gateway:
 - `ALTER COLUMN column_name SET|ADD FIELD field_definition`
 - `ALTER COLUMN column_name DROP FIELD field_name`
 - `ALTER COLUMN column_name SET FIELDS (...)`
-- `SET|ADD MAPPING (key = value)`
+- `SET|ADD MAPPING key = value`
 - `DROP MAPPING key`
-- `SET|ADD SETTING (key = value)`
+- `SET|ADD SETTING key = value`
 - `DROP SETTING key`
+- `SET|ADD ALIAS alias_name = value`
+- `DROP ALIAS alias_name`
+
+### Table-level clauses take no parentheses
+
+The table-level `MAPPING`, `SETTING` and `ALIAS` clauses are written as a bare `key = value`,
+**without** parentheses — unlike the column-level `ALTER COLUMN … SET|ADD OPTION (key = value)`:
+
+```sql
+ALTER TABLE orders (
+  SET MAPPING _meta.owner = 'analytics',
+  SET SETTING index.refresh_interval = '1s',
+  SET ALIAS recent_orders = (filter = (range = (order_date = (gte = 'now-7d'))))
+);
+```
+
+The parentheses around the statement list are optional and independent of any single clause —
+`ALTER TABLE orders SET SETTING index.refresh_interval = '1s'` is equally valid.
+
+A value may be a scalar (`'1s'`, `true`, `2`), an array (`['a', 'b']`), or a nested object
+written with **parentheses**: `(key = value, key = (nested = value))`. The `{…}` brace form is
+for `STRUCT` column values in `INSERT`, and is not accepted here.
 
 ### Type Changes and Safety
 
@@ -1391,7 +1439,7 @@ CREATE TABLE events (
   event_type KEYWORD,
   timestamp TIMESTAMP,
   PRIMARY KEY (id)
-) WITH (default_pipeline = "events_enriched");
+) OPTIONS (default_pipeline = 'events_enriched');
 ```
 
 #### 6. Insert data

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

@@ -55,6 +55,14 @@ package object sql {
     case _       => ""
   }
 
+  /** Escapes a bare string for a single-quoted SQL literal, exactly reversing how the grammar's
+    * `'([^'\\]|\\.)*'` unescapes it — backslash first, then the quote. Renderers that hold a plain
+    * `String` rather than a `StringValue` (a column COMMENT, for one) need it too, which is why it
+    * lives here rather than on the value.
+    */
+  def escapeStringLiteral(value: String): String =
+    value.replace("\\", "\\\\").replace("'", "\\'")
+
   /** Base trait for all tokens
     */
   trait Token extends Serializable with Validation {
@@ -438,42 +446,37 @@ package object sql {
       }
       .mkString("(", ", ", ")")
 
+    /** Recursion is on the HEAD of the path, one level at a time. Descending to the LEAF's parent
+      * and then re-attaching it under `keys.head` collapsed the intermediate levels: for a path of
+      * depth 3 or more, `_meta.columns.<c>.default_value` replaced the whole of `_meta` with the
+      * innermost object, dropping every sibling key. Depth 1 and 2 happened to be correct, which is
+      * why it survived — the metadata paths the schema writes are exactly the deeper ones.
+      */
     def set(path: String, newValue: Value[_]): ObjectValue = {
-      val keys = path.split("\\.")
-      val updatedValue = {
-        if (keys.length == 1) {
-          value + (keys.head -> newValue)
-        } else {
-          val parentPath = keys.dropRight(1).mkString(".")
-          val parentKey = keys.last
-          val parentObject = find(parentPath) match {
+      path.split("\\.").toList match {
+        case Nil      => this
+        case k :: Nil => ObjectValue(value + (k -> newValue))
+        case k :: rest =>
+          val child = value.get(k) match {
             case Some(obj: ObjectValue) => obj
             case _                      => ObjectValue.empty
           }
-          val updatedParent = parentObject.set(parentKey, newValue)
-          value + (keys.head -> updatedParent)
-        }
+          ObjectValue(value + (k -> child.set(rest.mkString("."), newValue)))
       }
-      ObjectValue(updatedValue)
     }
 
     def remove(path: String): ObjectValue = {
-      val keys = path.split("\\.")
-      val updatedValue = {
-        if (keys.length == 1) {
-          value - keys.head
-        } else {
-          val parentPath = keys.dropRight(1).mkString(".")
-          val parentKey = keys.last
-          val parentObject = find(parentPath) match {
-            case Some(obj: ObjectValue) => obj
-            case _                      => ObjectValue.empty
+      path.split("\\.").toList match {
+        case Nil      => this
+        case k :: Nil => ObjectValue(value - k)
+        case k :: rest =>
+          value.get(k) match {
+            case Some(obj: ObjectValue) =>
+              ObjectValue(value + (k -> obj.remove(rest.mkString("."))))
+            // nothing at that path: removing is a no-op, NOT a reason to overwrite `k`
+            case _ => this
           }
-          val updatedParent = parentObject.remove(parentKey)
-          value + (keys.head -> updatedParent)
-        }
       }
-      ObjectValue(updatedValue)
     }
 
     def find(path: String): Option[Value[_]] = {
@@ -575,7 +578,10 @@ package object sql {
   }
 
   case class StringValue(override val value: String) extends Value[String](value) {
-    override def sql: String = s"""'$value'"""
+    // Escaped exactly as the grammar's literal (`'([^'\\]|\\.)*'`) unescapes it — backslash first,
+    // then the quote, so the composition reverses. Without this a value holding an apostrophe
+    // rendered `'it's'`, which no longer parses.
+    override def sql: String = s"""'${escapeStringLiteral(value)}'"""
     override def baseType: SQLType = SQLTypes.Varchar
 
     override def ddl: String = s""""${value.replace("\\", "\\\\").replace("\"", "\\\"")}""""

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

@@ -188,19 +188,29 @@ object Parser
     (keyword("ALTER") ~ keyword("PIPELINE")) ~ ifExists ~ ident ~ start.? ~ repsep(
       alterPipelineStatement,
       separator
-    ) ~ end.? ^^ { case _ ~ ie ~ pipeline ~ s ~ stmts ~ e =>
+    ) ~ end.? >> { case _ ~ ie ~ pipeline ~ s ~ stmts ~ e =>
       if (s.isDefined && e.isEmpty) {
-        throw new Exception("Mismatched closing parentheses in ALTER PIPELINE statement")
+        err("Mismatched closing parentheses in ALTER PIPELINE statement")
       } else if (s.isEmpty && e.isDefined) {
-        throw new Exception("Mismatched opening parentheses in ALTER PIPELINE statement")
+        err("Mismatched opening parentheses in ALTER PIPELINE statement")
       } else if (s.isEmpty && e.isEmpty && stmts.size > 1) {
-        throw new Exception("Multiple ALTER PIPELINE statements require parentheses")
+        err("Multiple ALTER PIPELINE statements require parentheses")
       } else
-        AlterPipeline(pipeline, ie, stmts)
+        success(AlterPipeline(pipeline, ie, stmts))
     }
 
+  /** `FIELDS (…)` — required. The empty fallback belongs to `optionalMultiFields`, whose only
+    * caller is a column definition (a column need not declare sub-fields). Folding it in here made
+    * every consumer optional, and `alterColumnFields` (`ALTER COLUMN c SET <multiFields>`) then
+    * matched a bare `SET` with an empty field list: `ALTER COLUMN c SET` parsed as a no-op, and
+    * `ALTER COLUMN c SET FIELD raw KEYWORD` matched that same alternative and left `FIELD raw
+    * KEYWORD` unconsumed — so `SET FIELD` silently did nothing before #213 made trailing input an
+    * error, and could not parse at all afterwards.
+    */
   def multiFields: PackratParser[List[Column]] =
-    keyword("FIELDS") ~ start ~> repsep(column, separator) <~ end ^^ (cols => cols) | success(Nil)
+    keyword("FIELDS") ~ start ~> repsep(column, separator) <~ end ^^ (cols => cols)
+
+  def optionalMultiFields: PackratParser[List[Column]] = multiFields | success(Nil)
 
   def ifExists: PackratParser[Boolean] =
     opt(keyword("IF") ~ keyword("EXISTS")) ^^ {
@@ -220,10 +230,6 @@ object Parser
       case None    => false
     }
 
-  def ingest_id: PackratParser[Value[_]] = "_id" ^^ (_ => IdValue)
-
-  def ingest_timestamp: PackratParser[Value[_]] = "_ingest.timestamp" ^^ (_ => IngestTimestampValue)
-
   def defaultVal: PackratParser[Option[Value[_]]] =
     opt(keyword("DEFAULT") ~ (value | ingest_id | ingest_timestamp)) ^^ {
       case Some(_ ~ v) => Some(v)
@@ -245,7 +251,7 @@ object Parser
     (keyword("SCRIPT") ~ keyword("AS")) ~ start ~ scriptValue ~ end ^^ { case _ ~ _ ~ s ~ _ => s }
 
   def column: PackratParser[Column] =
-    ident ~ extension_type ~ (script | multiFields) ~ defaultVal ~ notNull ~ comment ~ (options | success(
+    ident ~ extension_type ~ (script | optionalMultiFields) ~ defaultVal ~ notNull ~ comment ~ (options | success(
       ListMap.empty[String, Value[_]]
     )) ^^ { case name ~ dt ~ mfs ~ dv ~ nn ~ ct ~ opts =>
       mfs match {
@@ -543,10 +549,17 @@ object Parser
       DropColumnScript(name, ifExists = ie)
     }
 
+  /** The value grammar must match `defaultVal`'s: a column declares `DEFAULT _ingest.timestamp` at
+    * CREATE time, so an ALTER that sets the same default on an existing column has to accept it
+    * too. It did not, which broke the `_last_updated` column the materialized-view machinery adds
+    * whenever that column already exists (`TableDiff` renders `ColumnDefaultSet` and the extension
+    * runs the rendered SQL).
+    */
   def alterColumnDefault: PackratParser[AlterColumnDefault] =
-    alterColumnIfExists ~ ident ~ (keyword("SET") ~ keyword("DEFAULT")) ~ value ^^ {
-      case ie ~ name ~ _ ~ dv =>
-        AlterColumnDefault(name, dv, ifExists = ie)
+    alterColumnIfExists ~ ident ~ (keyword("SET") ~ keyword(
+      "DEFAULT"
+    )) ~ (value | ingest_id | ingest_timestamp) ^^ { case ie ~ name ~ _ ~ dv =>
+      AlterColumnDefault(name, dv, ifExists = ie)
     }
 
   def dropColumnDefault: PackratParser[DropColumnDefault] =
@@ -631,15 +644,18 @@ object Parser
     (keyword("ALTER") ~ keyword("TABLE")) ~ ifExists ~ ident ~ start.? ~ repsep(
       alterTableStatement,
       separator
-    ) ~ end.? ^^ { case _ ~ ie ~ table ~ s ~ stmts ~ e =>
+    ) ~ end.? >> { case _ ~ ie ~ table ~ s ~ stmts ~ e =>
+      // `err`, not `throw`: these run inside a combinator, and `Parser.apply` is typed
+      // `Either[ParserError, Statement]` — a raw exception escapes that signature and only
+      // `GatewayApi` happens to wrap the call in `ElasticResult.attempt`.
       if (s.isDefined && e.isEmpty) {
-        throw new Exception("Mismatched closing parentheses in ALTER TABLE statement")
+        err("Mismatched closing parentheses in ALTER TABLE statement")
       } else if (s.isEmpty && e.isDefined) {
-        throw new Exception("Mismatched opening parentheses in ALTER TABLE statement")
+        err("Mismatched opening parentheses in ALTER TABLE statement")
       } else if (s.isEmpty && e.isEmpty && stmts.size > 1) {
-        throw new Exception("Multiple ALTER TABLE statements require parentheses")
+        err("Multiple ALTER TABLE statements require parentheses")
       } else
-        AlterTable(table, ie, stmts)
+        success(AlterTable(table, ie, stmts))
     }
 
   // Watcher parsers
@@ -661,31 +677,31 @@ object Parser
   def compareWatcherCondition: PackratParser[CompareWatcherCondition] =
     keyword("WHEN") ~> opt(not) ~ ident ~ comparison_operator ~ opt(value) ~ opt(
       dateMathScript
-    ) ^^ { case n ~ field ~ op ~ v ~ fun =>
+    ) >> { case n ~ field ~ op ~ v ~ fun =>
       val target_op =
         n match {
           case Some(_) => op.not
           case None    => op
         }
       v match {
         case Some(value) =>
-          CompareWatcherCondition(field, target_op, Left(value))
+          success(CompareWatcherCondition(field, target_op, Left(value)))
         case None =>
           fun match {
             case Some(f) if f.identifier.dependencies.isEmpty =>
-              CompareWatcherCondition(
-                field,
-                target_op,
-                Right(f.identifier.withFunctions(f +: f.identifier.functions))
+              success(
+                CompareWatcherCondition(
+                  field,
+                  target_op,
+                  Right(f.identifier.withFunctions(f +: f.identifier.functions))
+                )
               )
             case Some(_) =>
-              throw new Exception(
-                s"Date/datetime functions with field dependencies are not supported for comparison"
+              err(
+                "Date/datetime functions with field dependencies are not supported for comparison"
               )
             case None =>
-              throw new Exception(
-                s"A value or a date/datetime function must be provided for comparison"
-              )
+              err("A value or a date/datetime function must be provided for comparison")
           }
       }
     }
@@ -865,13 +881,10 @@ object Parser
     }
 
   def watcherAction: PackratParser[(String, WatcherAction)] =
-    ident ~ opt(keyword("AS")) ~ (loggingAction | webhookAction) ^^ { case name ~ _ ~ wa =>
+    ident ~ opt(keyword("AS")) ~ (loggingAction | webhookAction) >> { case name ~ _ ~ wa =>
       wa match {
-        case Some(wa) => (name, wa)
-        case _ =>
-          throw new Exception(
-            s"Unsupported watcher action type in action '$name'"
-          )
+        case Some(wa) => success((name, wa))
+        case _        => err(s"Unsupported watcher action type in action '$name'")
       }
     }
 
@@ -1324,12 +1337,16 @@ trait Parser
       ObjectValues(ovs)
     }
 
+  // `ingest_id | ingest_timestamp` for the same reason as `alterColumnDefault`: the mapping
+  // metadata a column's DEFAULT is mirrored into (`_meta.columns.<c>.default_value`) is written
+  // through this production.
   def option: PackratParser[(String, Value[_])] =
-    (ident | literal) ~ "=" ~ (objectValues | objectValue | value) ^^ { case key ~ _ ~ value =>
-      key match {
-        case lit: StringValue => (lit.value, value)
-        case id: String       => (id, value)
-      }
+    (ident | literal) ~ "=" ~ (objectValues | objectValue | value | ingest_id | ingest_timestamp) ^^ {
+      case key ~ _ ~ value =>
+        key match {
+          case lit: StringValue => (lit.value, value)
+          case id: String       => (id, value)
+        }
     }
 
   def options: PackratParser[ListMap[String, Value[_]]] =

sql/src/main/scala/app/softnetwork/elastic/sql/parser/type/package.scala

@@ -21,7 +21,9 @@ import app.softnetwork.elastic.sql.{
   BooleanValues,
   DoubleValue,
   DoubleValues,
+  IdValue,
   Identifier,
+  IngestTimestampValue,
   LongValue,
   LongValues,
   Null,
@@ -91,6 +93,16 @@ package object `type` {
     def value: PackratParser[Value[_]] =
       literal | pi | random | double | long | boolean | nullValue | param | array
 
+    /** The two ingest-time placeholders a column DEFAULT may carry. They live beside `value` so
+      * every value position can opt into them — `object Parser`'s `defaultVal` and `trait Parser`'s
+      * `option` both need them, and only the former could see them while they were declared in the
+      * object.
+      */
+    def ingest_id: PackratParser[Value[_]] = "_id" ^^ (_ => IdValue)
+
+    def ingest_timestamp: PackratParser[Value[_]] =
+      "_ingest.timestamp" ^^ (_ => IngestTimestampValue)
+
     def identifierWithValue: Parser[Identifier] = (value ^^ functionAsIdentifier) >> cast
 
     def char_type: PackratParser[SQLTypes.Char.type] =