Skip to content

Commit ece2def

Browse files
committed
fix sql script fields with aggregations, shouldBeScripted with arithmetic expressions, coalesce painless script
1 parent 8c9c744 commit ece2def

File tree

7 files changed

+23
-28
lines changed

7 files changed

+23
-28
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,6 @@ object ElasticAggregation {
282282
nested: Option[NestedElement],
283283
allElasticAggregations: Seq[ElasticAggregation]
284284
): Option[Aggregation] = {
285-
val nbBuckets = buckets.size
286285
buckets.reverse.foldLeft(Option.empty[Aggregation]) { (current, bucket) =>
287286
// Determine the bucketPath of the current bucket
288287
val currentBucketPath = bucket.identifier.path

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

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1918,7 +1918,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
19181918
| "c": {
19191919
| "script": {
19201920
| "lang": "painless",
1921-
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.minus(35, ChronoUnit.MINUTES)); def param2 = ZonedDateTime.now(ZoneId.of('Z')).toLocalDate(); param1 != null ? param1 : param2"
1921+
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.minus(35, ChronoUnit.MINUTES)); def param2 = ZonedDateTime.now(ZoneId.of('Z')).toLocalDate(); (param1 != null ? param1 : param2)"
19221922
| }
19231923
| }
19241924
| },
@@ -1952,6 +1952,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
19521952
.replaceAll("ChronoUnit", " ChronoUnit")
19531953
.replaceAll("=ZonedDateTime", " = ZonedDateTime")
19541954
.replaceAll(":ZonedDateTime", " : ZonedDateTime")
1955+
.replaceAll(";\\(param", "; (param")
19551956
}
19561957

19571958
it should "handle nullif function as script field" in {
@@ -1968,7 +1969,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
19681969
| "c": {
19691970
| "script": {
19701971
| "lang": "painless",
1971-
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); 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.now(ZoneId.of('Z')).toLocalDate(); param3 != null ? param3 : param4"
1972+
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); 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.now(ZoneId.of('Z')).toLocalDate(); (param3 != null ? param3 : param4)"
19721973
| }
19731974
| }
19741975
| },
@@ -2010,6 +2011,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
20102011
.replaceAll(",DateTimeFormatter", ", DateTimeFormatter")
20112012
.replaceAll("=ZonedDateTime", " = ZonedDateTime")
20122013
.replaceAll(":ZonedDateTime", " : ZonedDateTime")
2014+
.replaceAll(";\\(param", "; (param")
20132015
}
20142016

20152017
it should "handle cast function as script field" in {
@@ -2026,7 +2028,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
20262028
| "c": {
20272029
| "script": {
20282030
| "lang": "painless",
2029-
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); def param2 = LocalDate.parse(\"2025-09-11\", DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")); def param3 = param1 == null || param1.isEqual(param2) ? null : param1; def param4 = ZonedDateTime.now(ZoneId.of('Z')).toLocalDate().minus(2, ChronoUnit.HOURS); try { param3 != null ? param3 : param4 } catch (Exception e) { return null; }"
2031+
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); def param2 = LocalDate.parse(\"2025-09-11\", DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")); def param3 = param1 == null || param1.isEqual(param2) ? null : param1; def param4 = ZonedDateTime.now(ZoneId.of('Z')).toLocalDate().minus(2, ChronoUnit.HOURS); try { (param3 != null ? param3 : param4) } catch (Exception e) { return null; }"
20302032
| }
20312033
| },
20322034
| "c2": {
@@ -2094,6 +2096,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
20942096
.replaceAll(":ZonedDateTime", " : ZonedDateTime")
20952097
.replaceAll("try \\{", "try { ")
20962098
.replaceAll("} catch", " } catch")
2099+
.replaceAll(";\\(param", "; (param")
20972100
}
20982101

20992102
it should "handle case function as script field" in { // 40
@@ -2754,14 +2757,6 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
27542757
| "match_all": {}
27552758
| },
27562759
| "size": 0,
2757-
| "script_fields": {
2758-
| "hire_date": {
2759-
| "script": {
2760-
| "lang": "painless",
2761-
| "source": "def param1 = (doc['hire_date'].size() == 0 ? null : doc['hire_date'].value.toLocalDate()); param1"
2762-
| }
2763-
| }
2764-
| },
27652760
| "_source": false,
27662761
| "aggs": {
27672762
| "dept": {

es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,6 @@ object ElasticAggregation {
279279
nested: Option[NestedElement],
280280
allElasticAggregations: Seq[ElasticAggregation]
281281
): Option[Aggregation] = {
282-
val nbBuckets = buckets.size
283282
buckets.reverse.foldLeft(Option.empty[Aggregation]) { (current, bucket) =>
284283
// Determine the bucketPath of the current bucket
285284
val currentBucketPath = bucket.identifier.path

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

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1918,7 +1918,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
19181918
| "c": {
19191919
| "script": {
19201920
| "lang": "painless",
1921-
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.minus(35, ChronoUnit.MINUTES)); def param2 = ZonedDateTime.now(ZoneId.of('Z')).toLocalDate(); param1 != null ? param1 : param2"
1921+
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.minus(35, ChronoUnit.MINUTES)); def param2 = ZonedDateTime.now(ZoneId.of('Z')).toLocalDate(); (param1 != null ? param1 : param2)"
19221922
| }
19231923
| }
19241924
| },
@@ -1952,6 +1952,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
19521952
.replaceAll("ChronoUnit", " ChronoUnit")
19531953
.replaceAll("=ZonedDateTime", " = ZonedDateTime")
19541954
.replaceAll(":ZonedDateTime", " : ZonedDateTime")
1955+
.replaceAll(";\\(param", "; (param")
19551956
}
19561957

19571958
it should "handle nullif function as script field" in {
@@ -1968,7 +1969,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
19681969
| "c": {
19691970
| "script": {
19701971
| "lang": "painless",
1971-
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); 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.now(ZoneId.of('Z')).toLocalDate(); param3 != null ? param3 : param4"
1972+
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); 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.now(ZoneId.of('Z')).toLocalDate(); (param3 != null ? param3 : param4)"
19721973
| }
19731974
| }
19741975
| },
@@ -2010,6 +2011,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
20102011
.replaceAll(",DateTimeFormatter", ", DateTimeFormatter")
20112012
.replaceAll("=ZonedDateTime", " = ZonedDateTime")
20122013
.replaceAll(":ZonedDateTime", " : ZonedDateTime")
2014+
.replaceAll(";\\(param", "; (param")
20132015
}
20142016

20152017
it should "handle cast function as script field" in {
@@ -2026,7 +2028,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
20262028
| "c": {
20272029
| "script": {
20282030
| "lang": "painless",
2029-
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); def param2 = LocalDate.parse(\"2025-09-11\", DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")); def param3 = param1 == null || param1.isEqual(param2) ? null : param1; def param4 = ZonedDateTime.now(ZoneId.of('Z')).toLocalDate().minus(2, ChronoUnit.HOURS); try { param3 != null ? param3 : param4 } catch (Exception e) { return null; }"
2031+
| "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value.toLocalDate()); def param2 = LocalDate.parse(\"2025-09-11\", DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")); def param3 = param1 == null || param1.isEqual(param2) ? null : param1; def param4 = ZonedDateTime.now(ZoneId.of('Z')).toLocalDate().minus(2, ChronoUnit.HOURS); try { (param3 != null ? param3 : param4) } catch (Exception e) { return null; }"
20302032
| }
20312033
| },
20322034
| "c2": {
@@ -2094,6 +2096,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
20942096
.replaceAll(":ZonedDateTime", " : ZonedDateTime")
20952097
.replaceAll("try \\{", "try { ")
20962098
.replaceAll("} catch", " } catch")
2099+
.replaceAll(";\\(param", "; (param")
20972100
}
20982101

20992102
it should "handle case function as script field" in { // 40
@@ -2754,14 +2757,6 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers {
27542757
| "match_all": {}
27552758
| },
27562759
| "size": 0,
2757-
| "script_fields": {
2758-
| "hire_date": {
2759-
| "script": {
2760-
| "lang": "painless",
2761-
| "source": "def param1 = (doc['hire_date'].size() == 0 ? null : doc['hire_date'].value.toLocalDate()); param1"
2762-
| }
2763-
| }
2764-
| },
27652760
| "_source": false,
27662761
| "aggs": {
27672762
| "dept": {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ package object cond {
138138
callArgs
139139
.take(values.length - 1)
140140
.map { arg =>
141-
s"${arg.trim} != null ? ${arg.trim}" // TODO check when value is nullable and has functions
141+
s"(${arg.trim} != null ? ${arg.trim}" // TODO check when value is nullable and has functions
142142
}
143-
.mkString(" : ") + s" : ${callArgs.last}"
143+
.mkString(" : ") + s" : ${callArgs.last})"
144144
}
145145

146146
override def nullable: Boolean = values.forall(_.nullable)

sql/src/main/scala/app/softnetwork/elastic/sql/operator/math/ArithmeticExpression.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,4 +130,6 @@ case class ArithmeticExpression(
130130
}
131131

132132
override def hasAggregation: Boolean = left.hasAggregation || right.hasAggregation
133+
134+
override def shouldBeScripted: Boolean = true
133135
}

sql/src/main/scala/app/softnetwork/elastic/sql/query/SQLSearchRequest.scala

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ case class SQLSearchRequest(
3838
lazy val bucketNames: Map[String, Bucket] = buckets.flatMap { b =>
3939
val name = b.identifier.identifierName
4040
"\\d+".r.findFirstIn(name) match {
41-
case Some(n) =>
41+
case Some(n) if name.trim.split(" ").length == 1 =>
4242
val identifier = select.fields(n.toInt - 1).identifier
4343
val updated = b.copy(identifier = select.fields(n.toInt - 1).identifier)
4444
Map(
@@ -114,7 +114,12 @@ case class SQLSearchRequest(
114114
)
115115
}
116116

117-
lazy val scriptFields: Seq[Field] = select.fields.filter(_.isScriptField)
117+
lazy val scriptFields: Seq[Field] = {
118+
if (aggregates.nonEmpty)
119+
Seq.empty
120+
else
121+
select.fields.filter(_.isScriptField)
122+
}
118123

119124
lazy val fields: Seq[String] = {
120125
if (aggregates.isEmpty && buckets.isEmpty && bucketScripts.isEmpty)

0 commit comments

Comments
 (0)