fix(sql): seven of #373's nine Painless emission defects, plus a semicolon-in-a-literal residual

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

@@ -690,6 +690,23 @@ package object bridge {
       case _           => true
     }))
 
+  /** The same question asked of BOTH operands (issue #373).
+    *
+    * 🔴 `requiresScript` only ever inspected the LEFT identifier, so a comparison of one column
+    * with another -- which the ES query DSL cannot express at all -- fell through to `rangeQuery`
+    * keyed on the left field NAME, with the RIGHT column rendered as DATE MATH. MEASURED: `WHERE d
+    * > DATE_TRUNC(ts, MONTH)` emitted `{"range":{"d":{"gt":"ts||/M"}}}` and Elasticsearch answered
+    * `parse_exception: failed to parse date field [ts] with format
+    * [strict_date_optional_time||epoch_millis]` -- it read the column NAME as a date literal. A
+    * range can only compare a field with a CONSTANT; two fields need the script path, which renders
+    * both sides and already emits a correct comparison.
+    */
+  private[bridge] def requiresScript(identifier: Identifier, maybeValue: Option[Token]): Boolean =
+    requiresScript(identifier) || maybeValue.exists {
+      case id: Identifier => id.name.trim.nonEmpty
+      case _              => false
+    }
+
   private[bridge] def scriptQueryOf(criteria: Criteria)(implicit
     timestamp: Long,
     contextType: PainlessContextType
@@ -713,7 +730,7 @@ package object bridge {
     import expression._
     if (isAggregation)
       return matchAllQuery()
-    if (requiresScript(identifier)) return scriptQueryOf(expression)
+    if (requiresScript(identifier, maybeValue)) return scriptQueryOf(expression)
     // Geo distance special case
     identifier.functions.headOption match {
       case Some(d: Distance) =>
@@ -1016,6 +1033,10 @@ package object bridge {
     contextType: PainlessContextType = PainlessContextType.Query
   ): Query = {
     import between._
+    // ⚠️ NOT the two-operand form: `BetweenExpr.maybeValue` is always a `FromTo`, never an
+    // `Identifier`, so passing it here would be dead code. `d BETWEEN ts AND ts` therefore still
+    // fails -- LOUDLY, at query-build time (`Unsupported out type for range query: ANY`) -- and
+    // is recorded as a residual rather than silently half-fixed.
     if (requiresScript(identifier)) return scriptQueryOf(between)
     // Geo distance special case
     identifier.functions.headOption match {

bridge/src/test/scala/app/softnetwork/elastic/sql/ColumnVsColumnSpec.scala

@@ -0,0 +1,104 @@
+/*
+ * Copyright 2025 SOFTNETWORK
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package app.softnetwork.elastic.sql
+
+import app.softnetwork.elastic.sql.bridge._
+import app.softnetwork.elastic.sql.parser.Parser
+import app.softnetwork.elastic.sql.query._
+import app.softnetwork.elastic.sql.schema.{Column, Table}
+import app.softnetwork.elastic.sql.`type`.SQLTypes
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import java.time.ZonedDateTime
+
+/** Issue #373, item 4 — a comparison of one column with another must run as a SCRIPT.
+  *
+  * `requiresScript` only ever inspected the LEFT identifier, so a column-to-column comparison --
+  * which the Elasticsearch query DSL cannot express at all -- fell through to `rangeQuery` keyed on
+  * the left field NAME, with the RIGHT column rendered as DATE MATH. MEASURED on ES 8.18:
+  * `{"range":{"d":{"gt":"ts||/M"}}}` answered `failed to parse date field [ts] with format
+  * [strict_date_optional_time||epoch_millis]` -- Elasticsearch read the column NAME as a date
+  * literal. A range can only compare a field with a CONSTANT.
+  *
+  * Executed on a real index (`a` d=2025-01-01/ts=2025-01-01, `b` d=2025-01-05/ts=2025-01-04, `c`
+  * d=2025-01-05 with NO ts): before, the shard error above; after, `['b']`, which is the truth.
+  */
+class ColumnVsColumnSpec extends AnyFlatSpec with Matchers {
+
+  implicit def timestamp: Long = ZonedDateTime.parse("2025-12-31T00:00:00Z").toInstant.toEpochMilli
+
+  private val schema: Table = Table(
+    "t",
+    columns = List(
+      Column("d", SQLTypes.Date),
+      Column("ts", SQLTypes.Timestamp),
+      Column("name", SQLTypes.Keyword),
+      Column("n", SQLTypes.Int)
+    )
+  )
+
+  /** 🔴 A schema-CARRYING parse, and that is what makes this spec able to see the defect at all.
+    * Without one every identifier stays `Any`, the date-math branch is never taken, and BOTH the
+    * fixed and the unfixed engine emit a script -- the first version of this spec passed with the
+    * fix reverted for exactly that reason. It is #306's rule again: the no-schema rendering is not
+    * the one a live query takes.
+    */
+  private def queryOf(sql: String): String =
+    Parser(sql) match {
+      case Right(ss: SingleSearch) =>
+        val request: ElasticSearchRequest = ss.update(Some(schema))
+        request.query.replaceAll("\\s+", "")
+      case other => fail(s"[$sql] expected a SingleSearch, got $other")
+    }
+
+  "a comparison of one column with a function of another" should "run as a script" in {
+    val q = queryOf("SELECT name FROM t WHERE d > DATE_TRUNC(ts, MONTH)")
+    withClue(q) {
+      q should include("\"script\"")
+      q should not include "\"range\""
+      q should include("doc['ts']")
+      q should not include "ts||/M"
+    }
+  }
+
+  it should "run as a script for two bare columns too" in {
+    val q = queryOf("SELECT name FROM t WHERE d > ts")
+    withClue(q) {
+      q should include("\"script\"")
+      q should not include "\"range\""
+      q should not include "ts||"
+    }
+  }
+
+  "a comparison against a CONSTANT" should "still use the query DSL" in {
+    // The range/term/terms paths must not move: they are what makes a filter cheap.
+    queryOf("SELECT name FROM t WHERE d > '2025-01-01'") should include(
+      "\"range\":{\"d\":{\"gt\":\"2025-01-01\"}}"
+    )
+    queryOf("SELECT name FROM t WHERE n > 3") should include("\"range\":{\"n\":{\"gt\":3}}")
+    queryOf("SELECT name FROM t WHERE name = 'x'") should include("\"term\"")
+    queryOf("SELECT name FROM t WHERE n IN (1,2)") should include("\"terms\"")
+    queryOf("SELECT name FROM t WHERE d BETWEEN '2025-01-01' AND '2025-02-01'") should include(
+      "\"range\""
+    )
+  }
+
+  "a function-wrapped LEFT operand" should "keep running as a script" in {
+    queryOf("SELECT name FROM t WHERE UPPER(name) = 'X'") should include("\"script\"")
+  }
+}

bridge/src/test/scala/app/softnetwork/elastic/sql/SQLQuerySpec.scala

@@ -1692,7 +1692,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
         |          "max": {
         |            "script": {
         |              "lang": "painless",
-        |              "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); def param2 = (doc['updatedAt'].size() == 0 ? null : doc['updatedAt'].value.toInstant().atZone(ZoneId.of('Z')).toLocalDate()); def param3 = ((param1 == null) ? null : ZonedDateTime.parse(param1, new DateTimeFormatterBuilder().appendPattern(\"yyyy-MM-dd HH:mm:ss\").appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true).toFormatter().withZone(ZoneId.of('Z'))) != null ? (param1 == null) ? null : ZonedDateTime.parse(param1, new DateTimeFormatterBuilder().appendPattern(\"yyyy-MM-dd HH:mm:ss\").appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true).toFormatter().withZone(ZoneId.of('Z'))).toLocalDate() : null); (param1 == null || param2 == null) ? null : Long.valueOf(ChronoUnit.DAYS.between(param3, param2))"
+        |              "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); def param2 = (doc['updatedAt'].size() == 0 ? null : doc['updatedAt'].value.toInstant().atZone(ZoneId.of('Z')).toLocalDate()); def param3 = (param1 == null) ? null : ZonedDateTime.parse(param1, new DateTimeFormatterBuilder().appendPattern(\"yyyy-MM-dd HH:mm:ss\").appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true).toFormatter().withZone(ZoneId.of('Z'))); def param4 = (param3 != null ? param3.toLocalDate() : null); (param1 == null || param2 == null) ? null : Long.valueOf(ChronoUnit.DAYS.between(param4, param2))"
         |            }
         |          }
         |        }
@@ -2178,7 +2178,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
         |    "c": {
         |      "script": {
         |        "lang": "painless",
-        |        "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value); def param2 = LocalDate.parse(\"2025-09-11\", DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")).minus(2, ChronoUnit.DAYS); def param3 = param1 == null || param1.isEqual(param2) ? null : param1; def param4 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(params.__now__), ZoneId.of('Z')).toLocalDate(); (param3 != null ? param3 : param4)",
+        |        "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value); def param2 = LocalDate.parse(\"2025-09-11\", DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")).minus(2, ChronoUnit.DAYS); def param3 = param1 == null || (param2 != null && param1.isEqual(param2)) ? null : param1; def param4 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(params.__now__), ZoneId.of('Z')).toLocalDate(); (param3 != null ? param3 : param4)",
         |        "params": {
         |          "__now__": 1767139200000
         |        }
@@ -2339,7 +2339,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
         |    "c": {
         |      "script": {
         |        "lang": "painless",
-        |        "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value); def param2 = LocalDate.parse(\"2025-09-11\", DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")); def param3 = param1 == null || param1.isEqual(param2) ? null : param1; def param4 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(params.__now__), ZoneId.of('Z')).toLocalDate().minus(2, ChronoUnit.HOURS); def safe1 = null; try { safe1 = (param3 != null ? param3 : param4); } catch (Exception e) {} safe1",
+        |        "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value); def param2 = LocalDate.parse(\"2025-09-11\", DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")); def param3 = param1 == null || (param2 != null && param1.isEqual(param2)) ? null : param1; def param4 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(params.__now__), ZoneId.of('Z')).toLocalDate().minus(2, ChronoUnit.HOURS); def safe1 = null; try { safe1 = (param3 != null ? param3 : param4); } catch (Exception e) {} safe1",
         |        "params": {
         |          "__now__": 1767139200000
         |        }
@@ -2505,7 +2505,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
         |    "c": {
         |      "script": {
         |        "lang": "painless",
-        |        "source": "def param1 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(params.__now__), ZoneId.of('Z')).toLocalDate().minus(7, ChronoUnit.DAYS); def param2 = (doc['lastUpdated'].size() == 0 ? null : doc['lastUpdated'].value.toLocalDate().minus(3, ChronoUnit.DAYS)); def param3 = (doc['lastUpdated'].size() == 0 ? null : doc['lastUpdated'].value.toLocalDate()); def param4 = (doc['lastSeen'].size() == 0 ? null : doc['lastSeen'].value.toLocalDate()); def param5 = (doc['lastSeen'].size() == 0 ? null : doc['lastSeen'].value.toInstant().atZone(ZoneId.of('Z')).toLocalDate().plus(2, ChronoUnit.DAYS)); def param6 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); param1 != null && param1.isEqual(param2) ? param3 : param1 != null && param1.isEqual(param4) ? param5 : param6",
+        |        "source": "def param1 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(params.__now__), ZoneId.of('Z')).toLocalDate().minus(7, ChronoUnit.DAYS); def param2 = (doc['lastUpdated'].size() == 0 ? null : doc['lastUpdated'].value.toLocalDate().minus(3, ChronoUnit.DAYS)); def param3 = (doc['lastUpdated'].size() == 0 ? null : doc['lastUpdated'].value.toLocalDate()); def param4 = (doc['lastSeen'].size() == 0 ? null : doc['lastSeen'].value.toLocalDate()); def param5 = (doc['lastSeen'].size() == 0 ? null : doc['lastSeen'].value.toInstant().atZone(ZoneId.of('Z')).toLocalDate().plus(2, ChronoUnit.DAYS)); def param6 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); param1 != null && param2 != null && param1.isEqual(param2) ? param3 : param1 != null && param4 != null && param1.isEqual(param4) ? param5 : param6",
         |        "params": {
         |          "__now__": 1767139200000
         |        }
@@ -2738,7 +2738,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
         |    "__c7": {
         |      "script": {
         |        "lang": "painless",
-        |        "source": "def param1 = (doc['identifier'].size() == 0 ? null : doc['identifier'].value); def param2 = (doc['identifier2'].size() == 0 ? null : doc['identifier2'].value); def lv0 = ((param1 == null || param2 == null) ? null : (param1 * param2)); (lv0 == null) ? null : (lv0 - 10)"
+        |        "source": "def param1 = (doc['identifier'].size() == 0 ? null : doc['identifier'].value); def param2 = (doc['identifier2'].size() == 0 ? null : doc['identifier2'].value); def lv1 = ((param1 == null || param2 == null) ? null : (param1 * param2)); (lv1 == null) ? null : (lv1 - 10)"
         |      }
         |    }
         |  },