UPDATE with string literal generates broken painless script (silent null overwrite)

[fupelaqu]

Summary

UPDATE <table> SET <col> = '<string-literal>' is translated into an
ingest-pipeline painless script that reads the literal as a field
reference on the doc rather than emitting a string literal. The
resulting script always assigns null to the target column, and
ignore_failure = true swallows the underlying error so the update
appears to succeed.

Repro

CREATE TABLE IF NOT EXISTS customers (
  id INT,
  name VARCHAR,
  country KEYWORD,
  PRIMARY KEY (id)
);

INSERT INTO customers (id, name, country) VALUES (1, 'Alice', 'France');

UPDATE customers SET country = 'USA' WHERE id = 1;
-- ✅ Updated 1 documents in customers.  (looks successful)

SELECT id, name, country FROM customers WHERE id = 1;
-- name = 'Alice', country = NULL   ← BUG: expected country = 'USA'

Evidence

The pipeline emitted by the SQL → ingest-pipeline translation for the
UPDATE above (from a CI log against ES 7.17.29 via the REST client):

SCRIPT(
  description = "country ANY SCRIPT AS (USA)",
  lang = "painless",
  source = "def param1 = ctx.USA; ctx.country = param1",
  ignore_failure = true
)

The source should be the equivalent of ctx.country = 'USA'. Instead
it reads ctx.USA — i.e. it dereferences a field named USA on the
document, which doesn't exist. param1 becomes null, then
ctx.country = null overwrites the existing value.

Impact

• Silent data corruption. The UPDATE reports success
  (✅ Updated 1 documents in customers.) but the target column is
  set to null. There is no error surfaced to the caller because
  ignore_failure = true masks the painless undefined-field error.
• Test data setup pitfall. Found while writing test setup that
  relied on UPDATE ... SET country = 'USA' WHERE id = 1 to pin a
  customer's country. The follow-up query found 0 USA rows because the
  field had been nulled, not updated.
• Production risk. Any user code issuing string-literal UPDATEs is
  silently nulling the targeted column. Numeric and boolean literals
  may be unaffected (untested).

Expected behaviour

UPDATE <table> SET <col> = '<string-literal>' WHERE <pred> should
produce a painless script that assigns the literal as a string value
to ctx.<col> — equivalent to ctx.<col> = '<literal>'.

Workaround

Use DELETE FROM <table> WHERE <pred> followed by INSERT INTO ...
with the full row. INSERT correctly serialises string literals.

Notes for the fix

• Verify whether the same bug affects other literal types (numeric,
  boolean, date, null).
• Consider removing or narrowing ignore_failure = true on
  user-issued UPDATE pipelines so this class of script error stops
  being silent.

[fupelaqu]

Root cause analysis

The broken painless script is the symptom of an unsafe AST round-trip in the SQL dispatch path,
not of an incorrect translation rule. Tracing what actually happens for UPDATE customers SET country = 'USA' WHERE id = 1:

1. GatewayApi.run(sql) parses the SQL once. The parser correctly produces
   Update(table="customers", values=ListMap("country" -> StringValue("USA")), where=...).
2. GatewayApi.run(update: Update) then re-emits the AST back to SQL via Update.sql and
   passes that string to IndicesApi.updateByQuery(index, update.sql).
3. IndicesApi.updateByQuery re-parses that string inside parseQueryForUpdate.

The Update.sql renderer used value.value for literals:

case value: Value[_] => s"$k = ${value.value}"

For StringValue("USA"), value.value is the bare Scala string "USA" (no quotes); for
numerics it happens to coincide with value.sql. So after step 2 the re-emitted SQL was:

UPDATE customers SET country = USA WHERE id = 1

When that re-parses in step 3 the country = USA token sequence is no longer a literal — it
matches identifierWithValue and wraps the bare token in Identifier(name = "USA"). From
that point on:

• Update.values("country") is an Identifier, not a Value[_].

• Update.customPipeline's pattern match goes to the case script branch instead of
  case value: Value[_], so it emits a ScriptProcessor instead of a SetProcessor.

• ScriptProcessor.fromScript calls script.painless(Some(ctx)). For an Identifier,
  PainlessContext.addParam(identifier) declares def param1 = ctx.<name> and returns
  param1. The combined source becomes the now-familiar:

  def param1 = ctx.USA; ctx.country = param1
  

• ignore_failure = true swallows the resulting MissingFieldException, and ES happily
  assigns null to country.

The existing unit test (parse UPDATE in ParserSpec) passes because it inspects
values("name") straight off the first parse — it never round-trips through update.sql.

Fix (PR forthcoming)

Two complementary changes in elasticsql:

1. sql/.../query/package.scala — Update.sql now renders literals via value.sql
   instead of value.value. value.sql preserves quoting for StringValue while remaining
   identical for numerics, booleans, etc., so the round-trip is parser-stable. New regression
   test in ParserSpec asserts round-trip stability and that the resulting customPipeline
   takes the SetProcessor branch.

2. core/.../client/IndicesApi.scala — updateByQuery overload that accepts an already
   parsed Update AST directly. GatewayApi.run(update: Update) now calls this overload, so
   the SQL is parsed exactly once. The string-based overload is retained for callers
   submitting raw JSON. Both overloads share a private runUpdateByQuery helper for the
   post-parse pipeline (extract → WHERE → JSON → execute).

The minimal one-line value.sql fix is enough to make the reported case work; the no-re-parse
refactor is the structural fix that closes off the whole "AST → re-emit → re-parse" failure
mode for any future literal-rendering bug.

Still recommended (separate work)

• Audit other literal types for the same renderer bug surface (date / timestamp / binary in
  Update.sql, and similar in Insert.sql if applicable).
• Reconsider ignore_failure = true on user-issued UPDATE pipelines — silently nulling a
  column on script failure is the root reason this bug shipped undetected.