From 65287a0613e11748e8bdb0ca5aac743acf1ac139 Mon Sep 17 00:00:00 2001
From: Andrew Hutchings <andrew@linuxjedi.co.uk>
Date: Mon, 22 Oct 2018 17:56:49 +0100
Subject: [PATCH] MCOL-1826 Fix race in FLOAT/DOUBLE to string

In the FLOAT/DOUBLE to string conversions a class global string was used
to store the result. Unfortunately it is possible for an instance of
this class to be used by multiple threads of PrimProc simultaneously.
This would cause a race and data corruption or more likely a crash.

This fix passes a string object from the caller to use instead.
---
 utils/funcexp/func_concat.cpp    |  7 +++++--
 utils/funcexp/func_concat_ws.cpp | 10 +++++++---
 utils/funcexp/func_elt.cpp       |  4 +++-
 utils/funcexp/func_insert.cpp    |  8 +++++---
 utils/funcexp/func_repeat.cpp    |  4 +++-
 utils/funcexp/func_reverse.cpp   |  3 ++-
 utils/funcexp/functor_str.h      |  8 +++-----
 7 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/utils/funcexp/func_concat.cpp b/utils/funcexp/func_concat.cpp
index 744b1c8ea..dc07876cb 100644
--- a/utils/funcexp/func_concat.cpp
+++ b/utils/funcexp/func_concat.cpp
@@ -55,10 +55,13 @@ string Func_concat::getStrVal(Row& row,
 								bool& isNull,
 								CalpontSystemCatalog::ColType&)
 {
-	string ret = stringValue(parm[0], row, isNull);
+	string ret;
+    string tmp;
+    stringValue(parm[0], row, isNull, ret);
 
 	for ( unsigned int id = 1 ; id < parm.size() ; id++) {
-		ret.append( stringValue(parm[id], row, isNull) );
+		stringValue(parm[id], row, isNull, tmp);
+        ret.append(tmp);
 	}
 
 	return ret;
diff --git a/utils/funcexp/func_concat_ws.cpp b/utils/funcexp/func_concat_ws.cpp
index 7a236cfea..2718f20b3 100644
--- a/utils/funcexp/func_concat_ws.cpp
+++ b/utils/funcexp/func_concat_ws.cpp
@@ -49,7 +49,8 @@ string Func_concat_ws::getStrVal(Row& row,
 								bool& isNull,
 								CalpontSystemCatalog::ColType&)
 {
-	string delim = stringValue(parm[0], row, isNull);
+	string delim;
+    stringValue(parm[0], row, isNull, delim);
 	if (isNull)
 		return "";
 
@@ -62,7 +63,8 @@ string Func_concat_ws::getStrVal(Row& row,
 
 	for ( unsigned int id = 1 ; id < parm.size() ; id++) 
 	{
-		string tstr = stringValue(parm[id], row, isNull); 
+		string tstr;
+        stringValue(parm[id], row, isNull, tstr); 
 		if (isNull)
 		{
 			isNull = false;
@@ -88,9 +90,11 @@ string Func_concat_ws::getStrVal(Row& row,
 
 #else
 	string str;
+    string tmp;
 	for ( uint32_t i = 1 ; i < parm.size() ; i++) 
 	{
-		str += string(stringValue(parm[i], row, isNull).c_str()); 
+		string(stringValue(parm[i], row, isNull).c_str(), tmp);
+        str += tmp;
 
 		if (isNull)
 		{
diff --git a/utils/funcexp/func_elt.cpp b/utils/funcexp/func_elt.cpp
index 521fc5ea5..4bdda1e1f 100644
--- a/utils/funcexp/func_elt.cpp
+++ b/utils/funcexp/func_elt.cpp
@@ -95,7 +95,9 @@ string Func_elt::getStrVal(rowgroup::Row& row,
 		return "";
 	}
 
-	return stringValue(parm[number], row, isNull);
+    std::string ret;
+	stringValue(parm[number], row, isNull, ret);
+    return ret;
 
 }
 
diff --git a/utils/funcexp/func_insert.cpp b/utils/funcexp/func_insert.cpp
index 519f860bc..1c6750e69 100644
--- a/utils/funcexp/func_insert.cpp
+++ b/utils/funcexp/func_insert.cpp
@@ -86,12 +86,14 @@ std::string Func_insert::getStrVal(rowgroup::Row& row,
 						FunctionParm& fp,
 						bool& isNull,
 						execplan::CalpontSystemCatalog::ColType&)
-{	
-	const string& tstr = stringValue(fp[0], row, isNull);
+{
+	string tstr;
+    stringValue(fp[0], row, isNull, tstr);
 	if (isNull)
 		return "";
 
-	const string& tnewstr = stringValue(fp[3], row, isNull);
+	string tnewstr;
+    stringValue(fp[3], row, isNull, tnewstr);
 	if (isNull)
 		return "";
 
diff --git a/utils/funcexp/func_repeat.cpp b/utils/funcexp/func_repeat.cpp
index b962e6690..e08cb1fb4 100644
--- a/utils/funcexp/func_repeat.cpp
+++ b/utils/funcexp/func_repeat.cpp
@@ -61,7 +61,9 @@ std::string Func_repeat::getStrVal(rowgroup::Row& row,
 						bool& isNull,
 						execplan::CalpontSystemCatalog::ColType& op_ct)
 {
-	string str = stringValue(fp[0], row, isNull);
+	string str;
+
+    stringValue(fp[0], row, isNull, str);
 
 	if (str.empty() || str == "")
 		return "";
diff --git a/utils/funcexp/func_reverse.cpp b/utils/funcexp/func_reverse.cpp
index 41ffbb198..1ae8b9f68 100644
--- a/utils/funcexp/func_reverse.cpp
+++ b/utils/funcexp/func_reverse.cpp
@@ -65,7 +65,8 @@ std::string Func_reverse::getStrVal(rowgroup::Row& row,
 						bool& isNull,
 						execplan::CalpontSystemCatalog::ColType&)
 {
-	string str = stringValue(fp[0], row, isNull);
+	string str;
+    stringValue(fp[0], row, isNull, str);
 
 	// We used to reverse in the string buffer, but that doesn't
 	// work for all compilers as some re-use the buffer on simple
diff --git a/utils/funcexp/functor_str.h b/utils/funcexp/functor_str.h
index e3c97cca8..e54511b36 100644
--- a/utils/funcexp/functor_str.h
+++ b/utils/funcexp/functor_str.h
@@ -85,7 +85,7 @@ public:
 
 
 protected:
-	const std::string& stringValue(execplan::SPTP& fp, rowgroup::Row& row, bool& isNull)
+	const void stringValue(execplan::SPTP& fp, rowgroup::Row& row, bool& isNull, std::string& fFloatStr)
 	{
 		// Bug3788, use the shorter of fixed or scientific notation for floating point values.
 		// [ the default format in treenode.h is fixed-point notation ]
@@ -103,7 +103,8 @@ protected:
             break;
 
 			default:
-				return fp->data()->getStrVal(row, isNull);
+				fFloatStr = fp->data()->getStrVal(row, isNull);
+                return;
 			break;
 		}
         exponent = (int)floor(log10( fabs(floatVal)));
@@ -121,10 +122,7 @@ protected:
             fFloatStr += buf;
         }
 
-        return fFloatStr;
 	}
-
-	std::string fFloatStr;
 };