[systemds] 02/02: [SYSTEMDS-3343] Fix scalar value type handling in named lists

[mb...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mboehm7 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/systemds.git

commit 157d355f1f6a96a8b2324b152bf69de233f71962
Author: Matthias Boehm <mb...@gmail.com>
AuthorDate: Tue May 17 17:21:10 2022 +0200

    [SYSTEMDS-3343] Fix scalar value type handling in named lists
    
    Named lists with literal values, are converting these values to strings
    during instruction generation. During runtime these names are probed for
    existing variables and otherwise created as string objects. Robustness
    measures on operations (type promotion) and function calls (argument
    casting) ensured correct conversions in most situations. However,
    boolean arguments were converted to lower case strings, and the casts
    expected upper case TRUE/FALSE and thus silently passed wrong values.
    
    This patch fixes the construction of named lists to perform a best
    effort conversion into boolean and int objects, as well as the casts
    of strings to boolean to consider both external and internal values.
    
    Thanks to @Sebastian for catching this issue.
---
 .../cp/ParameterizedBuiltinCPInstruction.java           |  3 ++-
 .../runtime/instructions/cp/ScalarObjectFactory.java    | 17 +++++++++++++----
 .../sysds/runtime/instructions/cp/StringObject.java     |  4 +++-
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/main/java/org/apache/sysds/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java b/src/main/java/org/apache/sysds/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
index 02b1638504..d679a1291c 100644
--- a/src/main/java/org/apache/sysds/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
+++ b/src/main/java/org/apache/sysds/runtime/instructions/cp/ParameterizedBuiltinCPInstruction.java
@@ -412,7 +412,8 @@ public class ParameterizedBuiltinCPInstruction extends ComputationCPInstruction
 		else if(opcode.equals("nvlist")) {
 			// obtain all input data objects and names in insertion order
 			List<Data> data = params.values().stream()
-				.map(d -> ec.containsVariable(d) ? ec.getVariable(d) : new StringObject(d))
+				.map(d -> ec.containsVariable(d) ? ec.getVariable(d) :
+					ScalarObjectFactory.createScalarObject(d))
 				.collect(Collectors.toList());
 			List<String> names = new ArrayList<>(params.keySet());
 
diff --git a/src/main/java/org/apache/sysds/runtime/instructions/cp/ScalarObjectFactory.java b/src/main/java/org/apache/sysds/runtime/instructions/cp/ScalarObjectFactory.java
index 99297146e1..6e74620394 100644
--- a/src/main/java/org/apache/sysds/runtime/instructions/cp/ScalarObjectFactory.java
+++ b/src/main/java/org/apache/sysds/runtime/instructions/cp/ScalarObjectFactory.java
@@ -27,10 +27,19 @@ import org.apache.sysds.runtime.util.UtilFunctions;
 
 public abstract class ScalarObjectFactory
 {
+	public static ScalarObject createScalarObject(String value) {
+		//best effort parsing of specialized types
+		if( UtilFunctions.isBoolean(value) )
+			return new BooleanObject(Boolean.parseBoolean(value));
+		if( UtilFunctions.isIntegerNumber(value) )
+			return new IntObject(UtilFunctions.parseToLong(value));
+		return new StringObject(value);
+	}
+	
 	public static ScalarObject createScalarObject(ValueType vt, String value) {
 		switch( vt ) {
-			case INT64:     return new IntObject(UtilFunctions.parseToLong(value));
-			case FP64:  return new DoubleObject(Double.parseDouble(value));
+			case INT64:   return new IntObject(UtilFunctions.parseToLong(value));
+			case FP64:    return new DoubleObject(Double.parseDouble(value));
 			case BOOLEAN: return new BooleanObject(Boolean.parseBoolean(value));
 			case STRING:  return new StringObject(value);
 			default: throw new RuntimeException("Unsupported scalar value type: "+vt.name());
@@ -62,8 +71,8 @@ public abstract class ScalarObjectFactory
 	
 	public static ScalarObject createScalarObject(ValueType vt, ScalarObject so) {
 		switch( vt ) {
-			case FP64:  return castToDouble(so);
-			case INT64:     return castToLong(so);
+			case FP64:    return castToDouble(so);
+			case INT64:   return castToLong(so);
 			case BOOLEAN: return new BooleanObject(so.getBooleanValue());
 			case STRING:  return new StringObject(so.getStringValue());
 			default: throw new RuntimeException("Unsupported scalar value type: "+vt.name());
diff --git a/src/main/java/org/apache/sysds/runtime/instructions/cp/StringObject.java b/src/main/java/org/apache/sysds/runtime/instructions/cp/StringObject.java
index 00d0037c37..05282bc876 100644
--- a/src/main/java/org/apache/sysds/runtime/instructions/cp/StringObject.java
+++ b/src/main/java/org/apache/sysds/runtime/instructions/cp/StringObject.java
@@ -37,7 +37,9 @@ public class StringObject extends ScalarObject
 	
 	@Override
 	public boolean getBooleanValue() {
-		return "TRUE".equals(_value);
+		//robustness for internal conversions
+		return "TRUE".equals(_value)
+			|| "true".equals(_value);
 	}
 
 	@Override