Restore replaceChildren(), fix condition on serialization, and improve some code

Author: ivancea

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/function/scalar/BinaryScalarFunction.java

@@ -46,7 +46,7 @@ public final BinaryScalarFunction replaceChildren(List<Expression> newChildren)
         Expression newLeft = newChildren.get(0);
         Expression newRight = newChildren.get(1);
 
-        return replaceChildren(newLeft, newRight);
+        return left.equals(newLeft) && right.equals(newRight) ? this : replaceChildren(newLeft, newRight);
     }
 
     protected abstract BinaryScalarFunction replaceChildren(Expression newLeft, Expression newRight);

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java

@@ -538,7 +538,7 @@ protected final Expression buildDeepCopyOfFieldExpression(TestCaseSupplier.TestC
     }
 
     private Expression randomSerializeDeserialize(Expression expression) {
-        if (false && randomBoolean()) {
+        if (randomBoolean()) {
             return expression;
         }
 
@@ -561,8 +561,14 @@ protected Expression serializeDeserializeExpression(Expression expression) {
             in -> in.readNamedWriteable(Expression.class),
             testCase.getConfiguration() // The configuration query should be == to the source text of the function for this to work
         );
-        // Fields use synthetic sources, which can't be serialized. So we use the originals instead.
-        return newExpression.replaceChildrenSameSize(expression.children());
+
+        // Fields use synthetic sources, which can't be serialized. So we replace with the originals instead.
+        var dummyChildren = newExpression.children()
+            .stream()
+            .<Expression>map(c -> new Literal(Source.EMPTY, "anything that won't match any test case", c.dataType()))
+            .toList();
+        // We first replace them with other unrelated expressions to force a replace, as some replaceChildren() will check for equality
+        return newExpression.replaceChildrenSameSize(dummyChildren).replaceChildrenSameSize(expression.children());
     }
 
     protected final Expression buildLiteralExpression(TestCaseSupplier.TestCase testCase) {

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/AbstractConfigurationFunctionTestCase.java

@@ -35,13 +35,10 @@ public void testSerializationWithConfiguration() {
 
         assertSerialization(expr, config);
 
-        Configuration differentConfig;
-        do {
-            differentConfig = randomConfiguration();
-        } while (config.equals(differentConfig));
+        Configuration differentConfig = randomValueOtherThan(config, AbstractConfigurationFunctionTestCase::randomConfiguration);
 
         Expression differentExpr = buildWithConfiguration(testCase.getSource(), testCase.getDataAsFields(), differentConfig);
-        assertFalse(expr.equals(differentExpr));
+        assertNotEquals(expr, differentExpr);
     }
 
     private static Configuration randomConfiguration() {