From 5755dd256df8651a2570af3ad5392ec388d33f8b Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 21 Sep 2016 15:49:56 -0700 Subject: [PATCH] Fix String concatentation bug. --- .../painless/node/EAssignment.java | 4 ++-- .../org/elasticsearch/painless/StringTests.java | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java index 50b56505eb8..a018ec4822b 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/node/EAssignment.java @@ -262,8 +262,8 @@ public final class EAssignment extends AExpression { rhs.write(writer, globals); // write the bytecode for the rhs - if (!(rhs instanceof EBinary) || ((EBinary)rhs).cat) { - writer.writeAppendStrings(rhs.actual); // append the rhs's value unless it's also a concatenation + if (!(rhs instanceof EBinary) || !((EBinary)rhs).cat) { // check to see if the rhs has already done a concatenation + writer.writeAppendStrings(rhs.actual); // append the rhs's value since it's hasn't already } writer.writeToStrings(); // put the value for string concat onto the stack diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/StringTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/StringTests.java index 873e773b9a3..b5b3e2cfbf6 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/StringTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/StringTests.java @@ -21,7 +21,9 @@ package org.elasticsearch.painless; import static org.elasticsearch.painless.WriterConstants.MAX_INDY_STRING_CONCAT_ARGS; +import java.util.HashMap; import java.util.Locale; +import java.util.Map; public class StringTests extends ScriptTestCase { @@ -182,7 +184,7 @@ public class StringTests extends ScriptTestCase { }); assertTrue(expected.getMessage().contains("Cannot cast [String] with length greater than one to [char].")); } - + public void testDefConcat() { assertEquals("a" + (byte)2, exec("def x = 'a'; def y = (byte)2; return x + y")); assertEquals("a" + (short)2, exec("def x = 'a'; def y = (short)2; return x + y")); @@ -205,7 +207,7 @@ public class StringTests extends ScriptTestCase { exec("def x = null; def y = null; return x + y"); }); } - + public void testDefCompoundAssignment() { assertEquals("a" + (byte)2, exec("def x = 'a'; x += (byte)2; return x")); assertEquals("a" + (short)2, exec("def x = 'a'; x += (short)2; return x")); @@ -222,6 +224,17 @@ public class StringTests extends ScriptTestCase { }); } + public void testComplexCompoundAssignment() { + Map params = new HashMap<>(); + Map ctx = new HashMap<>(); + ctx.put("_id", "somerandomid"); + params.put("ctx", ctx); + + assertEquals("somerandomid.somerandomid", exec("ctx._id += '.' + ctx._id", params, false)); + assertEquals("somerandomid.somerandomid", exec("String x = 'somerandomid'; x += '.' + x")); + assertEquals("somerandomid.somerandomid", exec("def x = 'somerandomid'; x += '.' + x")); + } + public void testAppendStringIntoMap() { assertEquals("nullcat", exec("def a = new HashMap(); a.cat += 'cat'")); }